Hi Martin,

On 02/14/18 02:30, Martin Buchholz wrote:
Unlike most fixes to data races, this one should benefit performance.


Nice catch (for java-tsan tool)!

Although I think this race is benign. The code that synchronizes on 'this' is the following:

  46     private boolean hasBeenFinalized() {
  47         return (next == this);
  48     }

And the only place where 'next' is set to 'this' is in remove() and remove is called from synchronized (this) block too. So hasBeenFinalized() may see any or no writes of 'next' performed out of synchronized (this), but they are never writes of value 'this', so the outcome is always 'false' in this case. The only write to 'next' with value of 'this' happens while synchronized (this) and synchronized (lock) at the same time so this write synchronizes with the read of 'next' in hasBeenFinalized() as well as with all other writes of 'next'.

It may be that the intent was to refrain from using the shared 'lock' lock for the 2nd and subsequent calls to runFinalizer() and only use the more fine-grained 'this' lock in this case?

If someone was able to call runFinalizer() on the same instance in a loop he could prevent or slow-down normal processing of other Finalizer(s) if the shared 'lock' was always employed. What do you think?

Regards, Peter

Reply via email to