RFR: 8253429: Error reporting should report correct state of terminated/aborted threads

[Thomas Stüfe]

Hi David,

On Fri, Sep 25, 2020 at 7:06 AM David Holmes <david.holmes at oracle.com>
wrote:

> Hi Thomas,
>
> On 25/09/2020 2:31 pm, Thomas Stuefe wrote:
> > On Thu, 24 Sep 2020 18:14:10 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:
> >
> >> For some non-JavaThread, their object instances can outlast threads'
> lifespan. For example, we still can query/report
> >> thread's state after thread terminated.
> >> But the query/report currently returns wrong state. E.g. a terminated
> thread appears to be alive and seemly has valid
> >> thread stack, etc.
> >> This patch sets non-JavaThread's state to ZOMBIE just before it
> terminates, so that we can distinguish terminated
> >> thread from live thread.
> >> Also, thread should not report its SMR info, if it has terminated or it
> never started (thread->osthread() == NULL).
> >>
> >> Note: Java thread does not have such issue, its thread object is
> deleted before thread terminates.
> >
> > Hi Zhengyu,
> >
> > Could this expose us to a race?
> >
> > What I mean is: on Linux and BSD I see this code in os::create_thread()
> after thread creation and after the initial
> > handshake with the parent thread:
> >
> >> A pthread_create ...
> >> B initial handshake...
> >>
> >> C // Aborted due to thread limit being reached
> >>   if (state == ZOMBIE) {
> >>     thread->set_osthread(NULL);
> >>     delete osthread;
> >>     return false;
> >>   }
> >
> > (Weirdly enough we don't do (C) for Windows and AIX).
> >
> > I may be wrong do not find anywhere in the code where we set the ZOMBIE
> state today. So (C) may be just dead code now.
>
> Funnily enough it seems to be dead code that you created:
>
> https://bugs.openjdk.java.net/browse/JDK-8078513
>
> but mea culpa too as I missed it in the review. :)
>
>
Hah! Thanks for reminding me :)

Okay, that's still left over from pre-NPTL.

@Zhengyu: Can you remove these two sections too in os_bsd.cpp and
os_linux.cpp or should I do this in a separate patch?

Cheers Thomas


> Otherwise, yes this would be a theoretical race. So lets just delete the
> above.
>
> Cheers,
> David
>
> > With your patch, we now set the state to ZOMBIE in post_run. If child
> finishes and sets ZOMBIE state before parent
> > reaches (C) os::create_thread would return not false even though the
> child thread started correctly and ran through?
> >
> > -------------
> >
> > PR: https://git.openjdk.java.net/jdk/pull/341
> >
>