Hi Martin, On 02/15/2018 06:47 AM, Martin Buchholz wrote:
Embarrassed by my failure to take runFinalizersOnExit into account, I tried to redo the patch to be actually correct.
I think that your patch was correct the first time too, but in view of future removing the need for "pollFirst" from unfinalized list, the revised patch separates this logic entirely into the to-be-removed method so it is preferable.
http://cr.openjdk.java.net/~martin/webrevs/jdk/Finalizer-data-race/ even though we may succeed in making runFinalizersOnExit go away, and I'm not planning to submit any time soon. The runFinalizersOnExit case needs different mechanics for removing elements from "unfinalized". The simple invariant is that you need to run the finalizer whenever an element is unlinked from unfinalized.
...and the check for "already been finalized" needs to be performed only in deregisterAndRunFinalizer as you did in new patch. Once runAllFinalizers is removed, this check and marking code could be removed too.
Although not strictly necessary for correctness, for the time being, it would be nice to be consistent in marking the Finalizer object "already finalized" in both places. Either set both next and prev to this or next to this and prev to null.
Regards, Peter
