[
https://issues.apache.org/jira/browse/UIMA-6168?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17111398#comment-17111398
]
Marshall Schor commented on UIMA-6168:
--------------------------------------
There must be some use case I'm missing. This fix might be covering up some
other issue.
Here's what's supposed to happen, and why this code was removed in PR 20.
There are multiple cases where alternations to CAS FSs might need "protecting".
Protecting means that before the alteration, the FS is removed from all
indexes in all views where it is indexed. Then the alteration(s) happen. At
the end of the protection block, the FS is added back (to all the indexes) in
all the views.
The mechanism for doing this is to record the FSs that need to be added back,
and at the end, add them.
That record is kept in instances of FSsToBeAddedback. For some operations
(e.g. deserialization) a single instance of this class may suffice. For other
operations (users invoking protectIndexes in a try / finally block, for
example, see
[https://uima.apache.org/d/uimaj-current/references.html#ugr.ref.cas.updating_indexed_feature_structures]
)
the user code could, inside the body of the protected block, invoke another
protectIndexes, ending up with multiple nested blocks. Each one of these is
kept in a stack, rooted in the baseCAS in svd.fssTobeAddedback.
Each time the add back code is called (CASImpl.addbackModifiedFSs), it looks at
this stack, and sees if the instance of FSsTobeAddedback is the last one in the
stack, and if so, it removes it from the stack.
I think I see one use case which was forgotten in the previous fix. This was
the case where the user did the
{code:java}
try (AutoCloseable ac = my_cas.protectIndexes()) {
... arbitrary user code which updates features
which may be "keys" in one or more indexes
}{code}
In this case, the "close()" method of FSsTobeAddedback (need to look at the
subclasses) is called, and that needs to check and drop the last one. So the
fix is kind-of right, but misses the other cases, and needs to be moved up to
the close method I think. It should also do the previous impl, which has
additional checking to insure it's the last item in the chain:
{code:java}
int last_index = svd.fssTobeAddedback.size() -1;
if (last_index < 0) return;
FSsTobeAddedback last_one = svd.fssTobeAddedback.get(last_index);
if (fssTobeAddedback == last_one) {
svd.fssTobeAddedback.remove(last_index)
}
{code}
Do you want me to make this change to the pull request, or do you want to?
> protect indexes fails when there were no indexed fs that needed protecting
> --------------------------------------------------------------------------
>
> Key: UIMA-6168
> URL: https://issues.apache.org/jira/browse/UIMA-6168
> Project: UIMA
> Issue Type: Bug
> Components: Core Java Framework
> Affects Versions: 2.10.4SDK, 3.1.1SDK
> Reporter: Marshall Schor
> Assignee: Richard Eckart de Castilho
> Priority: Major
> Fix For: 3.2.0SDK, 2.10.5SDK
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> While testing Annotation trim (UIMA-6152) it had cases where protect indexes
> was used with no indexed items - and exposed an index out-of-bounds error in
> the actions taken for this case.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
