harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Pavel Afremov" <pavel.n.afre...@gmail.com>
Subject Re: [dlrvm] ClassCircularityError in recursive compilation (Was: Re: [drlvm] smoke test : gc PhantomReferenceQueueTest is really unstable)
Date Wed, 25 Oct 2006 06:52:55 GMT

The source code of the gc tests don't initiate circularity error directly.
It's unclear side effect, because error happen during VM start up time. In
this case additional printing for example can "fix" the bug. So I agree with
Gregory that tests shouldn't be changed in this situation in spite of it is
"normal", "synthetic" or contains errors.

Salikh do you check that before your changes in vm_hint_finalize, tests
gc.Finalizer and gc PhantomReferenceQueueTest crashed the VM, and after did
not? Your fix just switch off Finalization Work Balance Subsystem in spite
of the fact that circularity error happened in vm_enqueue_references() not
in finalizers thread start up . Your last changes can have the same effect
as all patch without regressions.

JNIEXPORT jint JNICALL Java_java_lang_FinalizerThread_doFinalization

   (JNIEnv *, jclass, jint quantity)


+    vm_enqueue_references();

     return (jint) vm_do_finalization(quantity);


So I try to fix circularity error without regression and now I am developing
a test which will reproduce circularity crash directly but not via side
effect. Of cause, it will be "synthetic" test, but it help us to developed a
real fix.

Pavel Afremov.

On 10/24/06, Salikh Zakirov <Salikh.Zakirov@intel.com> wrote:
> Gregory Shimansky wrote:
> > On Tuesday 24 October 2006 19:20 Salikh Zakirov wrote:
> >> I would like to request for comments on a possible way of getting rid
> >> of java execution from vm_hint_finalize(). The initial patch is
> attached to
> >> HARMONY-1952.
> >
> > You've also modified the problematic test in it. Are you sure this
> > modification doesn't change the test behavior in any way? Wouldn't it be
> > better to check that original test works well with it?
> The original test may not work with the modification. I consider this a
> bug in the test.
> It implicitly assumed that all cleared references will be available in
> reference queue
> as soon as System.gc() completes. This was true before.
> The patch in HARMONY-1952 breaks this, because references are enqueued
> from the context
> of finalization thread, and this may happen some time later.
> As the specification does not say at what exact moment cleared references
> should
> become available, I suppose that changing one implementation assumption
> ("references are available after System.gc()") to a weaker assumption
> ("references are available after System.gc();System.runFinalization()") is
> a correct thing to do.
> > Hmm... what if jthread_monitor_try_enter(finalizer_thread) fails, that
> is,
> > monitor is owned by another thread? Is the lock global for all
> finalization
> > threads? It won't give good results if there is more than one of them.
> Yes, the lock is global. I used try_enter() to prevent possible deadlock
> scenario,
> when the finalization happens at precisely the moment finalization thread
> is holding
> the finalization lock. If this happens, and vm_hint_finalize() cannot grab
> the finalization monitor, no harm is taken, because locked finalization
> monitor
> means that the finalization thread is active, and does not need to be
> waked up.
> On the other hand, the finalization queue itself is protected by a
> different native
> lock, which is guaranteed not to be locked at the garbage collection time
> (because it is locked and unlocked in suspend-disabled region).
> (* okay, I didn't check this, but I believe it should work this way *)
> > Other than that I think your approach with notifying finalizer threads
> instead
> > of running the code on the same stack is better.
> Let's wait for other potential problems to be found by other Harmony
> contributors :)
> In any case, it will take some time to bring this idea to a committable
> patch.

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message