Further (overnight) thinking on this issue: Locking the cas that getEmptyCas returns may break some existing user code.
And there's no need to protect users at this point against the actions that locking prevents, I think. So, I'm changing my approach - to keep getEmptyCas as it is - returning an unlocked CAS. However, I will put in the other change for releasing a CAS to unlock it before resetting it. I don't see any downside to that. -Marshall On 8/11/2011 5:56 PM, Marshall Schor wrote: > While working on a fix for UIMA-2211, I came across code in the core that > looks > wrong. > > There is code for getEmptyCas in UimaContext_ImplBase, that had comments to > the > effect that when the empty CAS was provided back to user code that requested > it > (typically within a Cas Multiplier), the Cas was "locked" (meaning that you > cannot reset it) and any class loader switching that might be needed for > loading > things related to the Cas (such as JCas cover classes) was done. The comment > said that when the "next()" method returned to the framework, the Cas would be > unlocked (and the class loaders restored). > > The scenario I think makes sense. The user code typically implements a > "next()" > method, called by the framework, to return another CAS, so the first thing > that > method does is get an empty one, then do some initialization, and the return > it. So while in the user code, the CAS should have the right class loaders > set > up for it, and be "locked" against operations that the user code shouldn't be > doing. > > However, even though the comments say this, the actual code that was there > called SwitchClassLoader (which skips locking the CAS) instead of > switchClassLoaderLockCasCL (which does the lock). So I think this should be > fixed. > > I changed it, and found some tests didn't run. One I traced to the code in > CasPool for releasing a CAS. It has code to unlock and restore the class > loaders, and reset the CAS, but the order of these were backwards in the > method > releaseCas(...) - - it was trying to reset the Cas before unlocking it...). > > Another testcase which now fails is testEnableReset in CasManager_implTest. > This test says that if a Cas is locked, you should not be able to release it > back to the CasPool, but now that I fixed the ordering, this does work. So I > think the test is wrong to expect releasing it to fail. > > ----------- > > Before I commit things, does anyone know of use cases where getEmptyCas should > *not* lock a CAS (meaning prevent users from calling reset on the cas? > > Thanks for thinking about this :-) -Marshall > >
