Looks good! /Staffan
On 18 jan 2013, at 14:45, Rickard Bäckman <rickard.back...@oracle.com> wrote: > Aleksey, > > thanks for your review! > > a) It was before on of my own changes used in os_solaris.cpp (I think, for > synchronization support for Suspend/Resume). > I don't think we wanted something external to mess with that lock. > > b) I've changed the indentation slightly. > Updated webrev at http://cr.openjdk.java.net/~rbackman/8006563.2/ (or at > least currently copying…) > > /R > > On Jan 18, 2013, at 2:12 PM, Aleksey Shipilev wrote: > >> On 01/18/2013 04:58 PM, Rickard Bäckman wrote: >>> http://cr.openjdk.java.net/~rbackman/8006563/ >> >> Looks good to me (not a Reviewer), modulo: >> a) Are we sure this thing is not acquired in some weird way, i.e. >> through JVMTI, SA, or whatnot? >> b) The formatting of the predicate does not follow it's structure, I >> think it should be: >> ... >> this != Interrupt_lock && >> !(this == Safepoint_lock && >> contains(locks, Terminator_lock) && >> SafepointSynchronize::is_synchronizing())) { >> >> This way it is more obvious SafepointSynchronize::is_synchronizing()) is >> the !(...) group. >> >> -Aleksey. >> >> >
