Skip to content

Commit

Permalink
Remove some synchronization in Reference to avoid deadlock
Browse files Browse the repository at this point in the history
Issue #13823

Signed-off-by: Peter Shipton <[email protected]>
  • Loading branch information
pshipton committed Nov 29, 2024
1 parent 752c652 commit 6b85f39
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 24 deletions.
30 changes: 14 additions & 16 deletions jcl/src/java.base/share/classes/java/lang/ref/Reference.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public abstract sealed class Reference<T> extends Object permits PhantomReferenc

private T referent;
private ReferenceQueue queue;
private int state;
private volatile int state;

/* jdk.lang.ref.disableClearBeforeEnqueue property allow reverting to the old behavior(non clear before enqueue)
* defer initializing the immutable variable to avoid bootstrap error
Expand Down Expand Up @@ -181,9 +181,7 @@ public T get() {
@Deprecated(since="16")
/*[ENDIF] JAVA_SPEC_VERSION >= 16 */
public boolean isEnqueued () {
synchronized(this) {
return state == STATE_ENQUEUED;
}
return state == STATE_ENQUEUED;
}

/**
Expand All @@ -206,14 +204,11 @@ boolean enqueueImpl() {
if (state == STATE_ENQUEUED || tempQueue == null) {
return false;
}
result = tempQueue.enqueue(this);
if (result) {
state = STATE_ENQUEUED;
if (null != tempReferent) {
reprocess();
}
tempQueue.enqueue(this);
if (null != tempReferent) {
reprocess();
}
return result;
return true;
}
}

Expand Down Expand Up @@ -252,13 +247,16 @@ void initReference (T r, ReferenceQueue q) {

/**
* Called when a Reference has been removed from its ReferenceQueue.
* Set the enqueued field to false.
*/
void dequeue() {
/*[PR 112508] not synchronized, so isEnqueued() could return wrong result */
synchronized(this) {
state = STATE_CLEARED;
}
state = STATE_CLEARED;
}

/**
* Called when a Reference has been added to it's ReferenceQueue.
*/
void setEnqueued() {
state = STATE_ENQUEUED;
}

/*[IF JAVA_SPEC_VERSION >= 9]*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,13 @@ public Reference<? extends T> remove(long timeout) throws IllegalArgumentExcepti
* true if reference is enqueued.
* false if reference failed to enqueue.
*/
boolean enqueue(Reference<? extends T> reference) {
void enqueue(Reference<? extends T> reference) {
/*[PR CMVC 96472] deadlock loading sun.misc.Cleaner */
/*[PR 102259] call Cleaner.clean(), do not enqueue */
if (reference instanceof Cleaner) {
reference.dequeue();
((Cleaner)reference).clean();
return true;
reference.setEnqueued();
}
/*[PR 125873] Improve reflection cache */
Class refClass = reference.getClass();
Expand All @@ -210,32 +210,32 @@ boolean enqueue(Reference<? extends T> reference) {
) {
reference.dequeue();
((Runnable)reference).run();
return true;
reference.setEnqueued();
}
synchronized(this) {
/*[PR CMVC 181985] Perf: zWAS ftprint regressed 6% Java7 vs 626FP1 -ReferenceQueue */
if ( references == null) {
if (references == null) {
references = new Reference[DEFAULT_QUEUE_SIZE];
} else if(!empty && head == tail) {
} else if (!empty && (head == tail)) {
/* Queue is full - grow */
int newQueueSize = (int)(references.length * 1.10);
Reference newQueue[] = new Reference[newQueueSize];
System.arraycopy(references, head, newQueue, 0, references.length - head);
if(tail > 0) {
if (tail > 0) {
System.arraycopy(references, 0, newQueue, references.length - head, tail);
}
head = 0;
tail = references.length;
references = newQueue;
}
references[tail++] = reference;
if(tail == references.length) {
if (tail == references.length) {
tail = 0;
}
empty = false;
reference.setEnqueued();
notifyAll();
}
return true;
}

void forEach(java.util.function.Consumer<? super Reference<? extends T>> consumer) {
Expand Down

0 comments on commit 6b85f39

Please sign in to comment.