Thank you, David.
I agree with you that proposed change would lead to race condition. I wonder,
is this possible to avoid deadlocks in class
initialization without sacrifice to performance?
As for poor design, sometimes base class may cause its subclass initialization.
For example,
enum Base {
DERIVED {};
}
Of cource, it's safe in the case of possible deadlocks. But anyway, I believe
that runtime should get round possible problems,
if it's possible and not hard to do. Even if it is really bad design, it
shouldn't worsen the situation falling into deadlock.
> Date: Wed, 25 Nov 2009 10:41:23 +1000
> From: david.hol...@sun.com
> Subject: Re: [Fwd: Deadlocked Thread State is RUNNABLE?]
> To: dmytro_she...@hotmail.com
> CC: serviceability-dev@openjdk.java.net; hotspot-runtime-...@openjdk.java.net
>
> Dmytro,
>
> Okay I see what you mean. However I'm more inclined to go with a design
> that benefits normal correct usage than to avoid a deadlock caused by
> poor design: a Base class should never depend on its subclasses during
> initialization.
>
> With the change you suggest, two threads trying to initialize the same
> class will race each other to the top of the hierarchy (or the first
> uninitialized class anyway) where one will perform the clinit and the
> other will wait. When clinit is done the waiting thread will be
> signalled and wakeup, both threads then race to initialize the next
> subclass - one will win and one will wait ... this process then
> continues down the hierarchy with the two threads "butting heads" at
> every step of the way. This is potentially a very wasteful process.
>
> Of course I can't speak to the original design and what considerations
> were made at that time - I wasn't there. This is just my opinion.
>
> Cheers,
> David Holmes
>
> Dmytro Sheyko said the following on 11/24/09 23:00:
> > David,
> >
> > > The lock of the class is not held during its own or its superclass's
> >
> > Yes, I know. I mean, let's say, artifical lock that prevents
> > simultaneous class initialization from different threads.
> >
> > The current scheme of class initialization is following:
> >
> > 1. Synchronize on the |Class| object that represents the class or
> > interface to be initialized. This involves waiting until the
> > current thread can obtain the lock for that object (§8.13)
> > <file:///C:/Tools/Java/docs/vmspec.2nded/Threads.doc.html#22500>.
> > 2. If initialization by some other thread is in progress for the
> > class or interface, then |wait| on this |Class| object (which
> > temporarily releases the lock). When the current thread awakens
> > from the |wait|, repeat this step.
> > 3. If initialization is in progress for the class or interface by the
> > current thread, then this must be a recursive request for
> > initialization. Release the lock on the |Class| object and
> > complete normally.
> > 4. If the class or interface has already been initialized, then no
> > further action is required. Release the lock on the |Class| object
> > and complete normally.
> > 5. If the |Class| object is in an erroneous state, then
> > initialization is not possible. Release the lock on the |Class|
> > object and throw a |NoClassDefFoundError|.
> > 6. Otherwise, record the fact that initialization of the |Class|
> > object is now in progress by the current thread and release the
> > lock on the |Class| object.
> > 7. Next, if the |Class| object represents a class rather than an
> > interface, and the direct superclass of this class has not yet
> > been initialized, then recursively perform this entire procedure
> > for the uninitialized superclass. If the initialization of the
> > direct superclass completes abruptly because of a thrown
> > exception, then lock this |Class| object, label it erroneous,
> > notify all waiting threads, release the lock, and complete
> > abruptly, throwing the same exception that resulted from the
> > initializing the superclass.
> > 8. Next, execute either the class variable initializers and static
> > initializers of the class or the field initializers of the
> > interface, in textual order, as though they were a single block,
> > except that |final| |static| variables and fields of interfaces
> > whose values are compile-time constants are initialized first.
> > 9. If the execution of the initializers completes normally, then lock
> > this |Class| object, label it fully initialized, notify all
> > waiting threads, release the lock, and complete this procedure
> > normally.
> > 10. Otherwise, the initializers must have completed abruptly by
> > throwing some exception E. If the class of E is not |Error| or one
> > of its subclasses, then create a new instance of the class
> > |ExceptionInInitializerError|, with E as the argument, and use
> > this object in place of E in the following step. But if a new
> > instance of |ExceptionInInitializerError| cannot be created
> > because an |OutOfMemoryError| occurs, then instead use an
> > |OutOfMemoryError| object in place of E in the following step.
> > 11. Lock the |Class| object, label it erroneous, notify all waiting
> > threads, release the lock, and complete this procedure abruptly
> > with reason E or its replacement as determined in the previous step.
> >
> >
> >
> > In this case if some thread started initialization of class DerivedA no
> > other thread can execute DerivedA.<clinit>.
> > I have following scheme in my mind (superclass initialization is moved up):
> >
> > 1. If the |Class| object represents a class rather than an interface,
> > and the direct superclass of this class has not yet been
> > initialized, then recursively perform this entire procedure for
> > the uninitialized superclass.
> > 2. Synchronize on the |Class| object that represents the class or
> > interface to be initialized. This involves waiting until the
> > current thread can obtain the lock for that object (§8.13)
> > <file:///C:/Tools/Java/docs/vmspec.2nded/Threads.doc.html#22500>.
> > 3. If initialization by some other thread is in progress for the
> > class or interface, then |wait| on this |Class| object (which
> > temporarily releases the lock). When the current thread awakens
> > from the |wait|, repeat this step.
> > 4. If initialization is in progress for the class or interface by the
> > current thread, then this must be a recursive request for
> > initialization. Release the lock on the |Class| object and
> > complete normally.
> > 5. If the class or interface has already been initialized, then no
> > further action is required. Release the lock on the |Class| object
> > and complete normally.
> > 6. If the |Class| object is in an erroneous state, then
> > initialization is not possible. Release the lock on the |Class|
> > object and throw a |NoClassDefFoundError|.
> > 7. Otherwise, record the fact that initialization of the |Class|
> > object is now in progress by the current thread and release the
> > lock on the |Class| object.
> > 8. Next, execute either the class variable initializers and static
> > initializers of the class or the field initializers of the
> > interface, in textual order, as though they were a single block,
> > except that |final| |static| variables and fields of interfaces
> > whose values are compile-time constants are initialized first.
> > 9. If the execution of the initializers completes normally, then lock
> > this |Class| object, label it fully initialized, notify all
> > waiting threads, release the lock, and complete this procedure
> > normally.
> > 10. Otherwise, the initializers must have completed abruptly by
> > throwing some exception E. If the class of E is not |Error| or one
> > of its subclasses, then create a new instance of the class
> > |ExceptionInInitializerError|, with E as the argument, and use
> > this object in place of E in the following step. But if a new
> > instance of |ExceptionInInitializerError| cannot be created
> > because an |OutOfMemoryError| occurs, then instead use an
> > |OutOfMemoryError| object in place of E in the following step.
> > 11. Lock the |Class| object, label it erroneous, notify all waiting
> > threads, release the lock, and complete this procedure abruptly
> > with reason E or its replacement as determined in the previous step.
> >
> >
> > Consider example:
> > class Base {
> > static {
> > DerivedB.init();
> > }
> > }
> > class DerivedA extends Base {}
> > class DerivedB extends Base {}
> >
> > ThreadA initializes DerivedA
> > ThreadB initializes DerivedB
> >
> > Existing scheme scenario:
> > 1. ThreadA marks DerivedA as being initialized by it (i.e. ThreadA).
> > 2. ThreadB marks DerivedB as being initialized by it (i.e. ThreadB).
> > 3. ThreadA starts initialization of class Base.
> > 4. ThreadB attempts to initialize class Base and waits for ThreadA.
> > 5. ThreadA invokes Base.<clinit>()
> > 6. ThreadA attempts to initialize class DerivedB and waits for ThreadB
> > (triggered by static method call).
> > --> deadlock.
> >
> > New scheme scenario:
> > 1. ThreadA starts initialization of class Base (superclass of DerivedA);
> > 2. ThreadB attempts to initialize class Base and waits for ThreadA.
> > 3. ThreadA invokes Base.<clinit>()
> > 4. ThreadA starts initialization of class DerivedB (triggered by static
> > method call)
> > 5. ThreadA starts initialization of class Base and it figures out that
> > is already being initialized by it - normal completion.
> > 6. ThreadA marks DerivedB as initialized;
> > 7. ThreadA marks Base as initialized;
> > 8. ThreadB wakes up and figures out that Base is already initialized;
> > 9. ThreadB figures out that DerivedB is already initialized;
> > 10. ThreadA marks DerivedA as initialized;
> >
> > --
> > Dmytro
> >
> > > Date: Tue, 24 Nov 2009 20:05:26 +1000
> > > From: david.hol...@sun.com
> > > Subject: Re: [Fwd: Deadlocked Thread State is RUNNABLE?]
> > > To: dmytro_she...@hotmail.com
> > > CC: serviceability-dev@openjdk.java.net;
> > hotspot-runtime-...@openjdk.java.net
> > >
> > > Dmytro Sheyko said the following on 11/24/09 19:42:
> > > > What is the reason to hold lock on class being initialized while
> > > > initializing its superclasses?
> > >
> > > The lock of the class is not held during its own or its superclass's
> > > initialization. A lock is only used to set the current class as
> > > being-initialized, and for concurrent initialization attempts to be able
> > > to wait.
> > >
> > > David Holmes
> > >
> > > > Wouldn't it better acquire lock after superclass initialization just
> > > > before calling static initializer?
> > > > This would prevents some kind of deadlocks.
> > > >
> > > > Thanks,
> > > > Dmytro
> > > >
> > > > > Date: Wed, 18 Nov 2009 11:58:18 +1000
> > > > > From: david.hol...@sun.com
> > > > > Subject: Re: [Fwd: Deadlocked Thread State is RUNNABLE?]
> > > > > To: mandy.ch...@sun.com
> > > > > CC: serviceability-dev@openjdk.java.net;
> > > > hotspot-runtime-...@openjdk.java.net
> > > > >
> > > > > Mandy Chung said the following on 11/18/09 11:36:
> > > > > > It's a known bug:
> > > > > >
> > > > > > 6501158: Thread state is incorrect during class initialization
> > > > > > procedure
> > > > > >
> > > > > > I recalled the discussion for this bug but don't remember if we
> > > > > > discussed enhancing the java.lang.management spec to cover
> > "waiting"
> > > > > > on VM internal actions.
> > > > > >
> > > > > > David will probably have more information about this.
> > > > >
> > > > > I have nothing really to add save what is stated in the CR, but as my
> > > > > main comment was not public I've moved it to being public (and
> > dropped
> > > > > myself as RE) and reproduce it below.
> > > > >
> > > > > Quite simply the code that does the "wait" is low-level in the VM and
> > > > > does not come through the normal Object.wait() path that would
> > set the
> > > > > Thread.State. It can be "fixed" but there are a couple of additional
> > > > > issues that also need to be addressed due to the fact that the
> > monitor
> > > > > used is not associated with Java-level object. (The JLS was
> > updated in
> > > > > this regard.)
> > > > >
> > > > > The meta-discussion was whether we should introduce a new
> > Thread.State
> > > > > to cover this special case (waiting for class initialization),
> > and that
> > > > > discussion seemed to lean towards doing this (I suggested it and
> > Mandy
> > > > > agreed it seemed like a good idea :) ) But things did not
> > progress from
> > > > > there.
> > > > >
> > > > > Cheers,
> > > > > David
> > > > > -----
> > > > >
> > > > > >From 6501158:
> > > > >
> > > > > The submitter quotes the JLS with regard to the class initialization
> > > > > procedure and how synchronization is employed. In fact hotspot
> > does not
> > > > > synchronize using the Class object monitor during class
> > initialization -
> > > > > this is to avoid denial-of-service style attacks just by explicitly
> > > > > locking a Class object. The JLS is in the process of being updated to
> > > > > say that a "unique initialization lock " is used for class
> > > > > initialization, not necessarily the Class object's lock. This
> > brings the
> > > > > spec into line with the hotspot implementation.
> > > > >
> > > > > The reason I mention this is that the monitor that hotspot uses is
> > > > > associated with the klassOop for the class. The monitor code sets
> > > > > current_waiting_monitor() or current_pending_monitor() as appropriate
> > > > > during wait() or monitor entry. The management code, via the
> > > > > ThreadService::ThreadSnapShot gets a hold of the object
> > associated with
> > > > > the monitor for a blocked thread and assumes that the object is
> > in fact
> > > > > the oop for a java.lang.Object. When the klassOop is treated as in
> > > > > instance oop and queried for its own class etc then we end up
> > crashing
> > > > > the VM.
> > > > >
> > > > > The suggested fix correctly sets the thread state to "WAITING":
> > > > >
> > > > > Full thread dump Java HotSpot(TM) Tiered VM
> > > > > (1.7.0-internal-dh198349-fastdebug mixed mode):
> > > > >
> > > > > "Runner" prio=3 tid=0x08171800 nid=0xb in Object.wait()
> > > > > [0xcb99d000..0xcb99dbb0]
> > > > > java.lang.Thread.State: WAITING (on object monitor)
> > > > >
> > > > > but additional changes are need in ThreadSnapShot to discard the
> > > > > non-instance oop. (It seems
> > JvmtiEnvBase::get_current_contended_monitor
> > > > > would need a similar modification). This seems to work and
> > getThreadInfo
> > > > > simply reports eg:
> > > > >
> > > > > Current thread info: "Runner" Id=8 WAITING
> > > > >
> > > > > which seems okay. And getLockInfo() returns null.
> > > > >
> > > > > It is unclear however whether reporting this information actually
> > > > > violates the specification for these management API's. A thread
> > is only
> > > > > WAITING when performing Object.wait(), in which case there must be an
> > > > > Object being waited upon and so LockInfo must return non-null
> > > > > information. Yet that is not the case here.
> > > > >
> > > > > It seems to me that while we can report the information above, it
> > might
> > > > > be better to see whether the management specification can be
> > enhanced to
> > > > > cover "waiting" on VM internal actions and to then report this
> > > > > circumstance as one of those.
> > > > >
> > > > > Note also that the existing hotspot code could already be
> > susceptible to
> > > > > a crash due to the use of the klassOop monitor for class
> > initialization.
> > > > > If the timing were just right, a call to getThreadInfo could see a
> > > > > thread blocked trying to acquire this monitor (not wait upon it) and
> > > > > that would be captured by the ThreadSnapshot and eventually cause a
> > > > > crash. The fact that the snapshot requires a safepoint makes it less
> > > > > likely that you would encounter the target thread while blocked
> > on the
> > > > > monitor, as the monitor is only held for a short period during class
> > > > > initialization.
> > > > >
> > > > > I will await discussion with the management/monitoring folk before
> > > > > deciding how best to proceed with this CR.
> > > > >
> > > >
> > > >
> > ------------------------------------------------------------------------
> > > > Windows Live: Keep your friends up to date with what you do online.
> > > >
> > <http://www.microsoft.com/middleeast/windows/windowslive/see-it-in-action/social-network-basics.aspx?ocid=PID23461::T:WLMTAGL:ON:WL:en-xm:SI_SB_1:092010>
> >
> > ------------------------------------------------------------------------
> > Windows Live Hotmail: Your friends can get your Facebook updates, right
> > from Hotmail®.
> > <http://www.microsoft.com/middleeast/windows/windowslive/see-it-in-action/social-network-basics.aspx?ocid=PID23461::T:WLMTAGL:ON:WL:en-xm:SI_SB_4:092009>
_________________________________________________________________
Windows Live: Make it easier for your friends to see what you’re up to on
Facebook.
http://www.microsoft.com/middleeast/windows/windowslive/see-it-in-action/social-network-basics.aspx?ocid=PID23461::T:WLMTAGL:ON:WL:en-xm:SI_SB_2:092009
