race condition in cglib MethodProxy.invokeSuper() causes wrong method invocation

45 views
Skip to first unread message

Matt Bastress

unread,
Apr 1, 2008, 4:12:50 PM4/1/08
to google-guice
We have been using Guice for method interception and have occasionally
encountered a bug that causes the wrong method to be invoked by the
method proxy. This either results in a ClassCastException or, in the
case of methods with compatible parameters, executing the wrong
method.

We tracked it down to MethodProxy.invokeSuper() in the version of
cglib that Guice uses (2.2_beta1). It is reproducible using multi-
threaded unit tests or in the debugger.

In invokeSuper(), threads race to do the init() and invoke the proxied
method:

if (f2 == null)
init();
return f2.invoke(i2, obj, args);

Because the first thread to init() initializes members in this order
(and not atomically):

f1 = helper(ci, ci.c1);
f2 = helper(ci, ci.c2);
i1 = f1.getIndex(sig1);
i2 = f2.getIndex(sig2);

the second thread in invokeSuper() can find that f2 is not null, and
call f2.invoke() with i2 == 0. This causes the wrong method to be
invoked.

Our fix was to initialize those four fields in init() as a single
volatile invariant and use the double-checked locking with volatile
that is safe with Java 5 and above.

I can make this fix available if anyone wants it.

Bob Lee

unread,
Apr 1, 2008, 4:32:10 PM4/1/08
to google...@googlegroups.com
You rock!

Josh McDonald

unread,
Apr 1, 2008, 7:54:49 PM4/1/08
to google...@googlegroups.com
Agreed, it would have taken aeons for me to track that down if I were having the same problem ;-)

-J
--
"Therefore, send not to know For whom the bell tolls, It tolls for thee."

:: Josh 'G-Funk' McDonald
:: 0437 221 380 :: jo...@gfunk007.com

AnonymousCodeholic

unread,
Apr 15, 2008, 2:18:34 AM4/15/08
to google-guice
I hit the same problem. IMHO this fills requirements for a bug, what
do you others think? I'll file a new issue to the issue tracker, if
anybody else doesn't.

> I can make this fix available if anyone wants it.

Please, do!

J

Matt Bastress

unread,
Apr 15, 2008, 12:08:23 PM4/15/08
to google-guice
I posted the modified file (with minimal changes--it is still ugly) as
a cglib patch:

http://sourceforge.net/tracker/index.php?func=detail&aid=1943116&group_id=56933&atid=482370

Incorporate the fix with the source for cglib used by Guice before you
build (it is different from the source available on sourceforge).

Matt

On Apr 15, 2:18 am, AnonymousCodeholic <juha.myntti...@gmail.com>
wrote:
Reply all
Reply to author
Forward
0 new messages