Skip to content

Commit

Permalink
8316337: (bf) Concurrency issue in DirectByteBuffer.Deallocator
Browse files Browse the repository at this point in the history
Reviewed-by: alanb, liach
  • Loading branch information
minborg committed Sep 19, 2023
1 parent 4461eeb commit cf74b8c
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 48 deletions.
22 changes: 3 additions & 19 deletions src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template
Original file line number Diff line number Diff line change
Expand Up @@ -75,31 +75,15 @@ class Direct$Type$Buffer$RW$$BO$

#if[byte]

private static class Deallocator
implements Runnable
{

private long address;
private long size;
private int capacity;

private Deallocator(long address, long size, int capacity) {
assert (address != 0);
this.address = address;
this.size = size;
this.capacity = capacity;
private record Deallocator(long address, long size, int capacity) implements Runnable {
private Deallocator {
assert address != 0;
}

public void run() {
if (address == 0) {
// Paranoia
return;
}
UNSAFE.freeMemory(address);
address = 0;
Bits.unreserveMemory(size, capacity);
}

}

private final Cleaner cleaner;
Expand Down
49 changes: 27 additions & 22 deletions src/java.base/share/classes/java/nio/MappedByteBuffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -116,28 +116,33 @@ public abstract sealed class MappedByteBuffer
}

UnmapperProxy unmapper() {
return fd != null ?
new UnmapperProxy() {
@Override
public long address() {
return address;
}

@Override
public FileDescriptor fileDescriptor() {
return fd;
}

@Override
public boolean isSync() {
return isSync;
}

@Override
public void unmap() {
Unsafe.getUnsafe().invokeCleaner(MappedByteBuffer.this);
}
} : null;
return fd == null
? null
: new UnmapperProxy() {

// Ensure safe publication as MappedByteBuffer.this.address is not final
private final long addr = address;

@Override
public long address() {
return addr;
}

@Override
public FileDescriptor fileDescriptor() {
return fd;
}

@Override
public boolean isSync() {
return isSync;
}

@Override
public void unmap() {
Unsafe.getUnsafe().invokeCleaner(MappedByteBuffer.this);
}
};
}

/**
Expand Down
11 changes: 4 additions & 7 deletions src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -1154,10 +1154,10 @@ private int writeInternal(ByteBuffer src, long position) throws IOException {

// -- Memory-mapped buffers --

private abstract static class Unmapper
private sealed abstract static class Unmapper
implements Runnable, UnmapperProxy
{
private volatile long address;
private final long address;
protected final long size;
protected final long cap;
private final FileDescriptor fd;
Expand Down Expand Up @@ -1194,10 +1194,7 @@ public long capacity() {
}

public void unmap() {
if (address == 0)
return;
nd.unmap(address, size);
address = 0;

// if this mapping has a valid file descriptor then we close it
if (fd.valid()) {
Expand All @@ -1214,7 +1211,7 @@ public void unmap() {
protected abstract void decrementStats();
}

private static class DefaultUnmapper extends Unmapper {
private static final class DefaultUnmapper extends Unmapper {

// keep track of non-sync mapped buffer usage
static volatile int count;
Expand Down Expand Up @@ -1247,7 +1244,7 @@ public boolean isSync() {
}
}

private static class SyncUnmapper extends Unmapper {
private static final class SyncUnmapper extends Unmapper {

// keep track of mapped buffer usage
static volatile int count;
Expand Down

1 comment on commit cf74b8c

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.