harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Evgueni Brevnov" <evgueni.brev...@gmail.com>
Subject Re: [drlvm][threading] Is it safe to use hythread_suspend_all and hythread_resume_all?
Date Thu, 19 Oct 2006 03:20:40 GMT
On 10/19/06, Weldon Washburn <weldonwjw@gmail.com> wrote:
> On 10/18/06, Evgueni Brevnov <evgueni.brevnov@gmail.com> wrote:
> >
> > On 10/17/06, Salikh Zakirov <Salikh.Zakirov@intel.com> wrote:
> > > Evgueni Brevnov wrote:
> > > > Hi All,
> > > >
> > > > I'd like to here you opinion regarding hythread_suspend_all and
> > > > hythread_resume_all functionality provided by TM. Actually I have to
> > > > separate questions:
> > > >
> > > > 1)  I found that hythread_suspend_all calls thread_safe_point_impl
> > > > inside. There is no assertion regarding thread's state upon entering
> > > > hythread_suspend_all. So it can be called in suspend disabled state
> > > > and nobody (at least me) expects to have a safe point during
> > > > hythread_suspend_all. The problem seems to be very similar with the
> > > > one discussed in "[drlvm][threading] Possible race condition in
> > > > implementation of conditional variables?" Your thoughts?
> > >
> > > The code you see is there to prevent following deadlock scenario:
> > >
> > >   Thread A           Thread B
> > >      |                  |
> > >      | <------------ suspend(A);
> > >      |           A->suspend_request = 1;
> > >      |        wait for A to reach a safepoint...
> > >      |                  |
> > >   suspend_all()         |
> > >  B->suspend_request = 1
> > > wait for B to reach a safepoint ...
> > >
> > > and then two threads are infinitely waiting one another.
> >
> > Salikh, I see your scenario...I don't suggest to remove safe points
> > from hythread_suspend_all. Contrary I believe it makes sense to
> > suspend other threads only if it suspender thread is in a safe region.
> > Agree?
>
>
> This seems to make the most sense.  I think there might be an argument that
> Thread A trying to suspend all other threads really does not have to be in
> suspend_enable mode.  But this corner case probably adds nothing but
> clutter/confusion to the design.  As a design rule, I believe any time a
> thread calls a function that might block, the thread should be in suspend
> enable mode.  This simple rule makes it much easier for the whole team
> working on the code base to know what to do.

Strongly agree!

>
> Suppose Thread A intends to suspend a subset of all java threads.  Thread A
> will need to block somehow and wait for the complete subset to get to
> suspended state with their stacks enumerable (suspend_enabled).  Meanwhile
> over on another CPU in the SMP box, a bunch of non-suspended threads chew up
> gobs of memory and one of them calls for a stop-the-world GC.  Ultimately
> Thread A as well as the subset that was suspended needs to be in a
> suspend_enabled state.
>
> The easiest sync model to reason about is one where Thread A suspends its
> target subset of java threads *before* allowing the stop-the-world gc to
> proceed.  A global thread/gc lock provides this guarantee.
>
> >
> > > > 2) Assume I need to suspend all threads in particular group. Ok I pass
> > > > that group to hythread_suspend_all. Later when all suspended threads
> > > > should be resumed I pass the same group to hythread_resume_all. But
> > > > threads were suspended group has changed. For example one new thread
> > > > was added to it. So the questions are. Is it acceptable to have such
> > > > "unsafe" functionality? Would it better to lock the group in
> > > > hythread_suspend_all and unlock it in hythread_resume_all.
> > >
> > > We may as well leave it as the responsibility of application / TI agent
> > > writer not to modify a suspended thread group.
> > > Why do you think this should be enforced?
> >
> > In general, any "good" design should strive to eliminate/minimize
> > cases of illegal use of the interface. In other words it should be
> > hard to use it in a buggy way. In our case that means it is better to
> > ensure integrity inside TM instead of making application responsible
> > for that .... if we can do it inside what is the reason not to do it?
>
>
> Yes.  Good idea provided it really can be done.
>
>
> Moreover if you look to the spec of hythread_suspend_all it states
> > "...This method sets a suspend request for the every thread in the
> > group and then returns the iterator that can be used to traverse
> > through the suspended threads..." But implementation contradicts with
> > that. If group is changed while you are traversing through the group
> > you can get wrong thread.
>
>
> Agreed.  What you describe can be a problem.  I would like to see a
> conservative, simple design.  Once we get the sync funtional part robust, we
> can look at the performance problems.  While it is possible to
> simultaneously walk a link-list while the list is changing, this adds way
> too much confusion at this stage of  VM development.  And the VM does not
> yet have enough performance where it make sense to look at such fine detail.
>
>
>  What is even worse you can get a crash when
> > one thread is iterating through the group while another thread inserts
> > new elements (or removes) to it.
>
>
> Actually a crash would be the best failure I can think of.  At least the
> crash occurs somewhere close to the bad code.  A worse scenario is that you
> can fool yourself into thinking you have processed all the threads on the
> list when,  in fact, you may have missed a couple.  And the impact may not
> surface for 10 seconds...
>
> Evgueni
> > >
> > >
> > > ---------------------------------------------------------------------
> > > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> > >
> > >
> >
> > ---------------------------------------------------------------------
> > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >
> >
>
>
> --
> Weldon Washburn
> Intel Middleware Products Division
>
>

---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org


Mime
View raw message