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] Should hythread_monitor_init() aquire the monitor?
Date Mon, 20 Nov 2006 05:38:19 GMT
Hi All,

Angela, I appreciate your comments....thanks

On 11/18/06, Angela Lin <alin.harmony@gmail.com> wrote:
> Some belated comments:
> 1. I agree that replacing the simple sem_wait() with the check for -1
> and EINTR is a good patch. The original writer of the code was working
> on a version of Linux that claimed sem_wait() would never return an
> error.

Agree. It also may be a good idea to assert that sem_wait doesn't
return EINVAL and EDEADLK.

>
> 2. I would additionally suggest that it might be a good idea to mask
> all async signals in the asynchSignalReporter thread.
>
> 3. Yes, we had bugs in mapUnix...() and mapPortLib...() functions.
> They had no defined return value for unexpected signals.

Will you take care about it?

>
> 4. To the best of my knowledge, the classlib doesn't intentionally use
> SIGUSR2 for anything. I don't have the code in front of me now, but if
> you grep for where masterASynchSignalHandler() is registered, you
> should see which signals get caught.

Ok, thanks.

>
> 5. If DRLVM uses SIGUSR2 heavily, you might consider extending the
> port library to handle that for you as well. The port library chains
> signal handlers. We needed to do this to defend ourselves against apps
> that would (natively) register their own signal handlers, then create
> the JVM and still expect their signal handlers to get called. I seem
> to remember that this was mentioned in another thread?

That's interesting. It seems DRLVM should be more friendly to user's
handlers :-). Does it make sense to put this task into TODO list... or
somewhere else?

Thanks
Evgueni

>
> Regards,
> Angela
>
> On 11/17/06, Gregory Shimansky <gshimansky@gmail.com> wrote:
> > Evgueni Brevnov wrote:
> > > In other words we will observe the crash as we do now if sem_wait
> > > completes unsuccessfully for whatever reason...
> >
> > Well it shouldn't return an error except for signal, shouldn't it? Two
> > possible other errors are EINVAL and EDEADLK which should never happen.
> >
> > Maybe we should add an assertion after it that sem_wait was successful
> > to catch this situation quickly, and it will be a good starting point
> > for investigation.
> >
> > > On 11/17/06, Evgueni Brevnov <evgueni.brevnov@gmail.com> wrote:
> > >> Gregory,
> > >>
> > >> The code which goes after sem_wait doesn't work properly if sem_wait
> > >> returns with an error code. So we need to either loop until sem_wait
> > >> returns successfully or adjust the code after sem_wait to handle
> > >> irregular cases.
> > >>
> > >> Thanks
> > >> Evgueni
> > >>
> > >> On 11/16/06, Geir Magnusson Jr. <geir@pobox.com> wrote:
> > >> > Yes - that's why I was poking him to see the patch.  I was going to
> > >> > suggest something very similar.
> > >> >
> > >> > geir
> > >> >
> > >> >
> > >> > Gregory Shimansky wrote:
> > >> > > Evgueni Brevnov wrote:
> > >> > >> You can look at the change here
> > >> > >> http://issues.apache.org/jira/browse/HARMONY-2203
> > >> > >
> > >> > > Could someone who knowns classlib native code internals better
> > >> than me
> > >> > > comment on this JIRA? I've added my comment from the general
POV.
> > >> > >
> > >> > > I would change the loop to detect only signal interruption like
> > >> > >
> > >> > > while (sem_wait(&wakeUpASynchReporter) == -1 && errno
== EINTR);
> > >> > >
> > >> > > Other than that I agree with the patch. I someone does not know,
> > >> every
> > >> > > step in gdb also interrupts sem_wait calls, so such loops are
a
> > >> common
> > >> > > practice when using semaphores.
> > >> > >
> > >> > > If someone knows classlib internal logic with this asynchronous
> > >> handlers
> > >> > > stuff please write your opinion.
> > >> > >
> > >> >
> > >>
> > >
> >
> >
> > --
> > Gregory
> >
> >
>

Mime
View raw message