Code review request: CR 7141259 Native stack is missing in hs_err

[David Holmes]

On 3/02/2012 2:11 AM, Zhengyu Gu wrote:
> The cause of the problem is that decode acquires lock inside signal
> handler and generates assertion.
>
> The decoder is only used by error handler at this point, so it is safe
> to run without lock. Locking has many undesirable consequences under
> this scenario, besides signal handler issue, the error handler can be
> invoked when the thread is holding other locks, as David Holmes suggested.
>
> But it will also be used in upcoming native memory tracking code, which
> potentially can have multi-thread invoking decoder, so synchronization
> is needed.
>
> The proposed fix is to let error handler set decoder to single threaded
> mode, in which mode no lock is acquired.

The comment here:

803   // This should be the only running thread, decoder can safely run in
  804   // single threaded mode without acquiring lock
  805   Decoder::set_single_threaded(true);
  806
  807   jlong mytid = os::current_thread_id();
  808   if (first_error == NULL &&
  809       Atomic::cmpxchg_ptr(this, &first_error, NULL) == NULL) {
  810
  811     // first time
  812     first_error_tid = mytid;
  813     set_error_reported();

isn't quite correct as at this stage you can have multiple threads 
trying to do report_and_die(). So at least move it inside the if block.

Later when there are multiple users of the Decoder I don't know how you 
will fix this because those other users can run concurrently with 
report_and_die(). That said I'm unclear what it is about the Decoder 
that requires locking - what state does it maintain? Is there a reason 
not to create a Decoder only when needed?

---

In decoder.cpp you can still use MutexLockerEx and as it will take a 
null mutex and just act as a no-op eg:

   MutexLockerEx locker( _single_thread ? NULL : _decoder_lock, true);

Cheers,
David
-----

>
> Please review
> Webrev: http://cr.openjdk.java.net/~zgu/7141259/webrev.00/
>
>
> Thanks,
>
> -Zhengyu
>