Bug 367667

Comment 1 Stephan Herrmann 2012-01-03 11:16:17 EST

I happen to see this in the debugger.

This is not a bcel bug but an inconsistency in the OTRE.

I can see that StaticSliceBaseTransformation.doTransformInterface(..)
has not worked (e.g., no field _OT$activeTeams generated), but during
doTransformCode(..) we come to the point where we assume that 
doTransformInterface(..) *did* work.

Theoretically I see these options:
(1) one of the following queries has changed its value between
    interface and code transformation:
      cg.isInterface()
      CallinBindingManager.isBoundBaseClass(class_name)
      CallinBindingManager.hasBoundBaseParent(class_name)
(2) someone else has added the class to state.interfaceTransformedClasses
(3) doTransformInterface() raised an exception that was swallowed upstream.

Of these, I'd ascribe highest probability to isBoundBaseClass(..) detecting
too late, that the class is indeed bound. At the point of the NPE
a correct RoleBaseBinding for the given base class exists in the
CallinBindingManager.

I'll add some harness to StaticSliceBaseTransformation...

BTW: the base class observed was RoleFileCache.

Comment 2 Stephan Herrmann 2012-01-03 16:44:12 EST

(In reply to comment #1)
> Theoretically I see these options:
> (1) one of the following queries has changed its value between
>     interface and code transformation:
>       cg.isInterface()
>       CallinBindingManager.isBoundBaseClass(class_name)
>       CallinBindingManager.hasBoundBaseParent(class_name)
> (2) someone else has added the class to state.interfaceTransformedClasses
> (3) doTransformInterface() raised an exception that was swallowed upstream.

While seeing the NPE consistently during debugging OT's CodeCompletionTest
I found that actually (2) is the culprit: the same class (RoleFileCache) 
was loaded concurrently by two threads. I then learned that class-name based
locking in Equinox only protects defineClass while
TransformerHook.processClass(..) is called from outside the locked section.

Now:
- thread 1 invokes doTransformInterface(Foo) and marks Foo as transformed
- thread 2 finds that Foo has already been transformed and skips
           interface transformation
- thread 2 calls doTransformCode(Foo) trying to add to an existing
           <clinit> but this only exists in thread 1 => NPE

Since different threads may concurrently process the same class (and only
the first thread defining the class wins) our transformers should actually
work more independently, too!

In the observed scenario this means that the following should not be shared:
- StaticSliceBaseTransformer.state.interfaceTransformedClasses
- TeamIdDispenser.instances

There may be more, which means we should rethink the whole story of
shared state.

(Note: for forcing the NPE in the given test, just set a breakpoint at
the end of StaticSliceBaseTransformer.doTransformInterface(..) with 
condition that class_name is RoleFileCache (fqn)).

Comment 3 Stephan Herrmann 2012-01-26 16:56:01 EST

Resolved by removing lots of shared (static) state in the OTRE.
Commit: ce3d0e3c5e62f04c7f840e5a0b0b55739eeae728

Comment 4 Stephan Herrmann 2012-09-22 15:55:50 EDT

The exception was never seen again, considering as fixed indeed.