RFR (S): 8024943: ciReplay: fails to dump replay data during safepointing

[John Rose]

On Oct 2, 2013, at 1:15 PM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:

> http://cr.openjdk.java.net/~vlivanov/8024943/webrev.00/
> 15 lines changed: 11 ins; 0 del; 4 mod
> 
> In some situations VM fails to dump compiler replay file (produces empty replay_pid%p.log). The reason is that ciEnv::dump_replay_data tries to do thread state transition to VM unconditionally and acquire Compile_lock. It doesn't always work in context of VM error reporter.
> 
> The fix is to provide new method specifically for VM error reporter (ciEnv::dump_replay_data_unsafe) which doesn't perform aforementioned actions.
> 
> Testing: manual (checked that a valid dump is produced for a crash during safepointing).

Is is possible that you've pushed a bug further into the CI code?  I would like you to take a look at this (if you haven't already) and either make code adjustments or describe current assumptions in comments, for the next maintainer.

What I mean about "pushed a bug" is that the CI is (generally speaking) sensitive to the thread state.  (Note the GUARDED_VM_ENTRY macros everywhere.)  The dump_replay_data function calls a bunch of dump_replay_data subroutines on lots of little data structures in the CI.  Are any of them also sensitive to thread state?  If so, perhaps you want to have your 

Also, the ciEnv::current() call returns something to the error reporter, and if that something is non-null, it is assumed to be (up to high probability, of course) a valid ciEnv structure.  Are there any places where the ciEnv guy is in an inconsistent state but still published by ciEnv::current()?  Probably not, but it might be worth a comment noting the appropriate conditions.  My specific worry is that the call to remove_symbols in ~ciEnv might leave a space where the dumper would not see valid state.

This crash dumper stuff cannot be nailed down 100%, of course, but this might be a good moment to make sure we've at least stomped on it a bit.

Reviewed, pending resolution of the above.

Thanks,
— John