jmx-dev [PATCH] JDK-7009998: JMX synchronization during connection restart is faulty

[shanliang]

The fix is OK for me.

The test is OK too.
I would like to test more, for example the first thread throws an 
IOException when executing the method doStart(), we can check whether 
the 2 other threads are used to execute doStart(). But this is out of 
this bug fix.

Thanks to fix this bug :)

Shanliang


Jaroslav Bachorik wrote:
> I am looking for a review and patch sponsor.
>
> Webrev available at
> http://cr.openjdk.java.net/~jbachorik/JDK-7009998/webrev.00
>
> The issue is about a possible race condition in the
> ClientCommunicatorAdmin when the reconnection process may be initiated
> by more than one thread (eg. 3). The main reason is that the
> re-connection routine logic is split into two synchronized blocks and it
> relies on the state staying consistent when transiting from the one
> synchronized block to the other.
>
> The race condition is described by the reporter as:
> "In reading the code there is a scenario where the synchronization does
> the wrong thing if 3 threads attempt to go through the code at the same
> time. Consider the code in
> com.sun.jmx.remote.internal.ClientCommunicatorAdmin.restart, the first
> thread will set the state to RE_CONNECTING and then leave the
> sychronization block, the second thread will find the state to be
> RE_CONNECTING and wait on the lock. When thread 1 finishes and sets the
> state to CONNECTED, then thread 2 can leave the synchronization block -
> but fails to set the state to RE_CONNECTING because that code is
> incorrectly in the else branch. Thus thread 2 starts the reconnecting
> and thread 3 wakes to find the state not RE_CONNECTING so it believes it
> can safely start the reconnect and it also starts reconnecting. The bad
> mode is discovered in the preReconnection method."
>
> The fix is adding a return statement at the end of the first
> synchronized block in case when the admin has been successfully
> re-connected by the other thread.
>
> Test in "test/com/sun/jmx/remote/CCAdminReconnectTest.java" tests the
> fix. Changes in "make/netbeans/jmx/build.properties" are there for the
> NetBeans project to recognize the newly added test.
>
>
> Thanks,
>
> -JB-
>
>