Hi Again,
While studying the code of Finalizer I found a strange discrepancy
between handling of 'unfinalized' field during processing of
Finalizer(s) taken from ReferenceQueue and taken from the 'unfinalized'
pointer itself (as in runAllFinalizers()). The remove() method is as
follows:
private void remove() {
synchronized (lock) {
if (unfinalized == this) {
if (this.next != null) {
unfinalized = this.next;
} else {
unfinalized = this.prev;
}
}
if (this.next != null) {
this.next.prev = this.prev;
}
if (this.prev != null) {
this.prev.next = this.next;
}
this.next = this; /* Indicates that this has been
finalized */
this.prev = this;
}
}
When Finalizer(s) are taken from ReferenceQueue, they are processed in
arbitrary order, so once in a while it can happen that the Finalizer at
the head of the list (pointed to by 'unfinalized') is processed this
way. The body of the 1st if statement in above method is executed in
such situation:
if (unfinalized == this) {
if (this.next != null) {
unfinalized = this.next;
} else {
unfinalized = this.prev;
}
}
But I can't figure out a situation when this.next would be null while
this.prev would be non-null. In other words, when 'unfinalized' would
not point to the head of the list. For comparison, when Finalizer(s) are
processed from the 'unfinalized' pointer directly (as in
runAllFinalizers()), the following is executed:
for (;;) {
Finalizer f;
synchronized (lock) {
f = unfinalized;
if (f == null) break;
unfinalized = f.next;
}
f.runFinalizer(jla);
}
... so it is assumed that the 'unfinalized' always points to the head.
If this was not true, not all Finalizer(s) would be processed.
So what I'm asking is whether this simplified if statement in remove()
would be equivalent:
if (unfinalized == this) {
unfinalized = this.next;
}
Or am I missing some situation?
Regards, Peter
On 02/14/2018 09:19 AM, Peter Levart wrote:
Hi Martin,
On 02/14/2018 07:58 AM, Peter Levart wrote:
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?
I checked all uses of runFinalizer() and I don't think it can be
called more than twice for the same Finalizer instance (for example if
there is a race between runAllFinalizers() and processing of
Finalizers taken from the ReferenceQueue). So your patch is a nice
simplification.
Regards, Peter
