On Mon, 14 Sep 2020 18:51:31 GMT, Daniel D. Daugherty <dcu...@openjdk.org> wrote:
>> @kimbarrett >> >>> src/hotspot/share/runtime/synchronizer.cpp >>> >>> The old code in chk_in_use_entry seems wrong. It checked for a null >>> object() and recorded that as an error. But then it went on and attempted >>> to use it as if it was not null. That's been fixed by the change. However, >>> the change no longer treats a null as an error. Probably this is because >>> it's weak, and so could have become null. But is that really possible for >>> an "in use" monitor? >> >> In the old code, an ObjectMonitor's object field should never be NULL when >> that ObjectMonitor is on an in-use list. We'll get the logging message and >> then a crash. I used to have guarantee(n->object() != NULL, ...) in there, >> but Robbin convinced me that was a waste because we'll just crash on >> the use of the NULL pointer and that was good enough. >> >> As Erik has already explained, now that we use a weak handle, the object >> can be GC'ed before the deflater thread comes along and removes the >> idle ObjectMonitor from the in-use list. > > @kimbarrett - I believe I've addressed your comments in this push: > https://github.com/openjdk/jdk/pull/135/commits/9fa2bed109d6fe352c610b26ada650226ce9cec4 @coleenp, @rkennke, and @kimbarrett - I believe all of the changes that you have requested have been made. Please confirm by re-reviewing. @dholmes-ora - I don't think you asked for any specific code changes. I've taken a first pass at creating a CSR: JDK-8253121 migrate ObjectMonitor::_object to OopStorage https://bugs.openjdk.java.net/browse/JDK-8253121 Please look it over and feel free to edit as needed. Since I don't do CSR's often, what I've done might be all wrong. :-) ------------- PR: https://git.openjdk.java.net/jdk/pull/135
