Fix drop race with Cleaner
Motivation: We were seeing rare test failures where a cleaner had raced to close a memory segment we were using or closing. The cause was that a single MemorySegment ended up used in multiple Buf instances. When the SizeClassedMemoryPool was closed, the memory segments could be disposed without closing the gate in the NativeMemoryCleanerDrop. The gate is important because it prevents double-frees of the memory segment. Modification: The fix is to change how the SizeClassedMemoryPool is closed, such that it always releases memory by calling `close()` on its buffers, which in turn will close the gate. The program will then proceed through the SizeClassedMemoryPool.drop implementation, which in turn will observe that the allocator is closed, and *then* dispose of the memory. Result: We should hopefully not see any more random test failures, but if we do, they would at least indicate a different bug. This particular one was mostly showing up inside the cleaner threads, which were ignoring the exception, but occasionally I think the race went the other way, causing a test failure.
This commit is contained in:
parent
2f99ee64a4
commit
2ce8c7dc18
@ -61,9 +61,9 @@ class NativeMemoryCleanerDrop implements Drop<Buf> {
|
|||||||
var mem = manager.unwrapRecoverableMemory(buf);
|
var mem = manager.unwrapRecoverableMemory(buf);
|
||||||
var delegate = this.delegate;
|
var delegate = this.delegate;
|
||||||
WeakReference<Buf> ref = new WeakReference<>(buf);
|
WeakReference<Buf> ref = new WeakReference<>(buf);
|
||||||
AtomicBoolean gate = new AtomicBoolean();
|
AtomicBoolean gate = new AtomicBoolean(true);
|
||||||
cleanable = new GatedCleanable(gate, CLEANER.register(this, () -> {
|
cleanable = new GatedCleanable(gate, CLEANER.register(this, () -> {
|
||||||
if (gate.compareAndSet(false, true)) {
|
if (gate.getAndSet(false)) {
|
||||||
Buf b = ref.get();
|
Buf b = ref.get();
|
||||||
if (b == null) {
|
if (b == null) {
|
||||||
pool.recoverMemory(mem);
|
pool.recoverMemory(mem);
|
||||||
@ -84,7 +84,7 @@ class NativeMemoryCleanerDrop implements Drop<Buf> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
public void disable() {
|
public void disable() {
|
||||||
gate.set(true);
|
gate.set(false);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
@ -70,7 +70,7 @@ class SizeClassedMemoryPool implements Allocator, AllocatorControl, Drop<Buf> {
|
|||||||
Send<Buf> send;
|
Send<Buf> send;
|
||||||
while ((send = v.poll()) != null) {
|
while ((send = v.poll()) != null) {
|
||||||
try {
|
try {
|
||||||
dispose(send.receive());
|
send.receive().close();
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
capturedExceptions.add(e);
|
capturedExceptions.add(e);
|
||||||
}
|
}
|
||||||
@ -86,12 +86,16 @@ class SizeClassedMemoryPool implements Allocator, AllocatorControl, Drop<Buf> {
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void drop(Buf buf) {
|
public void drop(Buf buf) {
|
||||||
|
if (closed) {
|
||||||
|
dispose(buf);
|
||||||
|
return;
|
||||||
|
}
|
||||||
var sizeClassPool = getSizeClassPool(buf.capacity());
|
var sizeClassPool = getSizeClassPool(buf.capacity());
|
||||||
sizeClassPool.offer(buf.send());
|
sizeClassPool.offer(buf.send());
|
||||||
if (closed) {
|
if (closed) {
|
||||||
Send<Buf> send;
|
Send<Buf> send;
|
||||||
while ((send = sizeClassPool.poll()) != null) {
|
while ((send = sizeClassPool.poll()) != null) {
|
||||||
dispose(send.receive());
|
send.receive().close();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user