Skip to content

Commit 5967aaf

Browse files
plevartmcimadamore
authored andcommittedMay 29, 2020
8246050: Improve scalability of MemoryScope
Reiplement memory scope using StampedLock Reviewed-by: psandoz
1 parent 4708c6d commit 5967aaf

File tree

6 files changed

+291
-111
lines changed

6 files changed

+291
-111
lines changed
 

‎src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/AbstractMemorySegmentImpl.java

+23-29
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
package jdk.internal.foreign;
2727

2828
import jdk.incubator.foreign.MemoryAddress;
29+
import jdk.incubator.foreign.MemoryLayout;
30+
import jdk.incubator.foreign.MemoryLayouts;
2931
import jdk.incubator.foreign.MemorySegment;
3032
import jdk.incubator.foreign.SequenceLayout;
3133
import jdk.internal.access.JavaNioAccess;
@@ -68,22 +70,20 @@ public abstract class AbstractMemorySegmentImpl implements MemorySegment, Memory
6870

6971
final long length;
7072
final int mask;
71-
final Thread owner;
7273
final MemoryScope scope;
7374

7475
@ForceInline
75-
AbstractMemorySegmentImpl(long length, int mask, Thread owner, MemoryScope scope) {
76+
AbstractMemorySegmentImpl(long length, int mask, MemoryScope scope) {
7677
this.length = length;
7778
this.mask = mask;
78-
this.owner = owner;
7979
this.scope = scope;
8080
}
8181

8282
abstract long min();
8383

8484
abstract Object base();
8585

86-
abstract AbstractMemorySegmentImpl dup(long offset, long size, int mask, Thread owner, MemoryScope scope);
86+
abstract AbstractMemorySegmentImpl dup(long offset, long size, int mask, MemoryScope scope);
8787

8888
abstract ByteBuffer makeByteBuffer();
8989

@@ -100,7 +100,7 @@ public AbstractMemorySegmentImpl asSlice(long offset, long newSize) {
100100
}
101101

102102
private AbstractMemorySegmentImpl asSliceNoCheck(long offset, long newSize) {
103-
return dup(offset, newSize, mask, owner, scope);
103+
return dup(offset, newSize, mask, scope);
104104
}
105105

106106
@SuppressWarnings("unchecked")
@@ -145,12 +145,12 @@ public final long byteSize() {
145145

146146
@Override
147147
public final boolean isAlive() {
148-
return scope.isAliveThreadSafe();
148+
return scope.isAlive();
149149
}
150150

151151
@Override
152152
public Thread ownerThread() {
153-
return owner;
153+
return scope.ownerThread();
154154
}
155155

156156
@Override
@@ -159,7 +159,7 @@ public AbstractMemorySegmentImpl withAccessModes(int accessModes) {
159159
if ((~accessModes() & accessModes) != 0) {
160160
throw new IllegalArgumentException("Cannot acquire more access modes");
161161
}
162-
return dup(0, length, (mask & ~ACCESS_MASK) | accessModes, owner, scope);
162+
return dup(0, length, (mask & ~ACCESS_MASK) | accessModes, scope);
163163
}
164164

165165
@Override
@@ -177,17 +177,16 @@ private void checkAccessModes(int accessModes) {
177177
@Override
178178
public MemorySegment withOwnerThread(Thread newOwner) {
179179
Objects.requireNonNull(newOwner);
180-
checkValidState();
181180
if (!isSet(HANDOFF)) {
182181
throw unsupportedAccessMode(HANDOFF);
183182
}
184-
if (owner == newOwner) {
183+
if (scope.ownerThread() == newOwner) {
185184
throw new IllegalArgumentException("Segment already owned by thread: " + newOwner);
186185
} else {
187186
try {
188-
return dup(0L, length, mask, newOwner, scope.dup());
187+
return dup(0L, length, mask, scope.dup(newOwner));
189188
} finally {
190-
//flush read/writes to memory before returning the new segment
189+
//flush read/writes to segment memory before returning the new segment
191190
VarHandle.fullFence();
192191
}
193192
}
@@ -198,19 +197,18 @@ public final void close() {
198197
if (!isSet(CLOSE)) {
199198
throw unsupportedAccessMode(CLOSE);
200199
}
201-
checkValidState();
202200
closeNoCheck();
203201
}
204202

205203
private final void closeNoCheck() {
206-
scope.close(true);
204+
scope.close();
207205
}
208206

209207
final AbstractMemorySegmentImpl acquire() {
210208
if (Thread.currentThread() != ownerThread() && !isSet(ACQUIRE)) {
211209
throw unsupportedAccessMode(ACQUIRE);
212210
}
213-
return dup(0, length, mask, Thread.currentThread(), scope.acquire());
211+
return dup(0, length, mask, scope.acquire());
214212
}
215213

216214
@Override
@@ -227,7 +225,7 @@ boolean isSmall() {
227225
}
228226

229227
void checkRange(long offset, long length, boolean writeAccess) {
230-
checkValidState();
228+
scope.checkValidState();
231229
if (writeAccess && !isSet(WRITE)) {
232230
throw unsupportedAccessMode(WRITE);
233231
} else if (!writeAccess && !isSet(READ)) {
@@ -238,10 +236,7 @@ void checkRange(long offset, long length, boolean writeAccess) {
238236

239237
@Override
240238
public final void checkValidState() {
241-
if (owner != null && owner != Thread.currentThread()) {
242-
throw new IllegalStateException("Attempt to access segment outside owning thread");
243-
}
244-
scope.checkAliveConfined();
239+
scope.checkValidState();
245240
}
246241

247242
// Helper methods
@@ -415,29 +410,28 @@ public static AbstractMemorySegmentImpl ofBuffer(ByteBuffer bb) {
415410
AbstractMemorySegmentImpl bufferSegment = (AbstractMemorySegmentImpl)nioAccess.bufferSegment(bb);
416411
final MemoryScope bufferScope;
417412
int modes;
418-
final Thread owner;
419413
if (bufferSegment != null) {
420414
bufferScope = bufferSegment.scope;
421415
modes = bufferSegment.mask;
422-
owner = bufferSegment.owner;
423416
} else {
424-
bufferScope = new MemoryScope(bb, null);
417+
bufferScope = MemoryScope.create(bb, null);
425418
modes = defaultAccessModes(size);
426-
owner = Thread.currentThread();
427419
}
428420
if (bb.isReadOnly()) {
429421
modes &= ~WRITE;
430422
}
431423
if (base != null) {
432-
return new HeapMemorySegmentImpl<>(bbAddress + pos, () -> (byte[])base, size, modes, owner, bufferScope);
424+
return new HeapMemorySegmentImpl<>(bbAddress + pos, () -> (byte[])base, size, modes, bufferScope);
433425
} else if (unmapper == null) {
434-
return new NativeMemorySegmentImpl(bbAddress + pos, size, modes, owner, bufferScope);
426+
return new NativeMemorySegmentImpl(bbAddress + pos, size, modes, bufferScope);
435427
} else {
436-
return new MappedMemorySegmentImpl(bbAddress + pos, unmapper, size, modes, owner, bufferScope);
428+
return new MappedMemorySegmentImpl(bbAddress + pos, unmapper, size, modes, bufferScope);
437429
}
438430
}
439431

440-
public static AbstractMemorySegmentImpl NOTHING = new AbstractMemorySegmentImpl(0, 0, null, MemoryScope.GLOBAL) {
432+
public static final AbstractMemorySegmentImpl NOTHING = new AbstractMemorySegmentImpl(
433+
0, 0, MemoryScope.createUnchecked(null, null, null)
434+
) {
441435
@Override
442436
ByteBuffer makeByteBuffer() {
443437
throw new UnsupportedOperationException();
@@ -454,7 +448,7 @@ Object base() {
454448
}
455449

456450
@Override
457-
AbstractMemorySegmentImpl dup(long offset, long size, int mask, Thread owner, MemoryScope scope) {
451+
AbstractMemorySegmentImpl dup(long offset, long size, int mask, MemoryScope scope) {
458452
throw new UnsupportedOperationException();
459453
}
460454
};

‎src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/HeapMemorySegmentImpl.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ public class HeapMemorySegmentImpl<H> extends AbstractMemorySegmentImpl {
5252
final Supplier<H> baseProvider;
5353

5454
@ForceInline
55-
HeapMemorySegmentImpl(long offset, Supplier<H> baseProvider, long length, int mask, Thread owner, MemoryScope scope) {
56-
super(length, mask, owner, scope);
55+
HeapMemorySegmentImpl(long offset, Supplier<H> baseProvider, long length, int mask, MemoryScope scope) {
56+
super(length, mask, scope);
5757
this.offset = offset;
5858
this.baseProvider = baseProvider;
5959
}
@@ -69,8 +69,8 @@ long min() {
6969
}
7070

7171
@Override
72-
HeapMemorySegmentImpl<H> dup(long offset, long size, int mask, Thread owner, MemoryScope scope) {
73-
return new HeapMemorySegmentImpl<H>(this.offset + offset, baseProvider, size, mask, owner, scope);
72+
HeapMemorySegmentImpl<H> dup(long offset, long size, int mask, MemoryScope scope) {
73+
return new HeapMemorySegmentImpl<>(this.offset + offset, baseProvider, size, mask, scope);
7474
}
7575

7676
@Override
@@ -121,7 +121,7 @@ public static MemorySegment makeArraySegment(double[] arr) {
121121

122122
static <Z> HeapMemorySegmentImpl<Z> makeHeapSegment(Supplier<Z> obj, int length, int base, int scale) {
123123
int byteSize = length * scale;
124-
MemoryScope scope = new MemoryScope(null, null);
125-
return new HeapMemorySegmentImpl<>(base, obj, byteSize, defaultAccessModes(byteSize), Thread.currentThread(), scope);
124+
MemoryScope scope = MemoryScope.create(null, null);
125+
return new HeapMemorySegmentImpl<>(base, obj, byteSize, defaultAccessModes(byteSize), scope);
126126
}
127127
}

‎src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/MappedMemorySegmentImpl.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ public class MappedMemorySegmentImpl extends NativeMemorySegmentImpl implements
4848

4949
private final UnmapperProxy unmapper;
5050

51-
MappedMemorySegmentImpl(long min, UnmapperProxy unmapper, long length, int mask, Thread owner, MemoryScope scope) {
52-
super(min, length, mask, owner, scope);
51+
MappedMemorySegmentImpl(long min, UnmapperProxy unmapper, long length, int mask, MemoryScope scope) {
52+
super(min, length, mask, scope);
5353
this.unmapper = unmapper;
5454
}
5555

@@ -60,8 +60,8 @@ ByteBuffer makeByteBuffer() {
6060
}
6161

6262
@Override
63-
MappedMemorySegmentImpl dup(long offset, long size, int mask, Thread owner, MemoryScope scope) {
64-
return new MappedMemorySegmentImpl(min + offset, unmapper, size, mask, owner, scope);
63+
MappedMemorySegmentImpl dup(long offset, long size, int mask, MemoryScope scope) {
64+
return new MappedMemorySegmentImpl(min + offset, unmapper, size, mask, scope);
6565
}
6666

6767
// mapped segment methods
@@ -103,13 +103,13 @@ public static MappedMemorySegment makeMappedSegment(Path path, long bytesSize, F
103103
if (bytesSize <= 0) throw new IllegalArgumentException("Requested bytes size must be > 0.");
104104
try (FileChannelImpl channelImpl = (FileChannelImpl)FileChannel.open(path, openOptions(mapMode))) {
105105
UnmapperProxy unmapperProxy = channelImpl.mapInternal(mapMode, 0L, bytesSize);
106-
MemoryScope scope = new MemoryScope(null, unmapperProxy::unmap);
106+
MemoryScope scope = MemoryScope.create(null, unmapperProxy::unmap);
107107
int modes = defaultAccessModes(bytesSize);
108108
if (mapMode == FileChannel.MapMode.READ_ONLY) {
109109
modes &= ~WRITE;
110110
}
111111
return new MappedMemorySegmentImpl(unmapperProxy.address(), unmapperProxy, bytesSize,
112-
modes, Thread.currentThread(), scope);
112+
modes, scope);
113113
}
114114
}
115115

‎src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/MemoryScope.java

+245-59
Original file line numberDiff line numberDiff line change
@@ -26,101 +26,287 @@
2626

2727
package jdk.internal.foreign;
2828

29+
import jdk.internal.vm.annotation.ForceInline;
30+
2931
import java.lang.invoke.MethodHandles;
3032
import java.lang.invoke.VarHandle;
33+
import java.util.Objects;
34+
import java.util.concurrent.atomic.LongAdder;
35+
import java.util.concurrent.locks.StampedLock;
3136

3237
/**
33-
* This class manages the temporal bounds associated with a memory segment. A scope has a liveness bit, which is updated
34-
* when the scope is closed (this operation is triggered by {@link AbstractMemorySegmentImpl#close()}). Furthermore, a scope is
35-
* associated with an <em>atomic</em> counter which can be incremented (upon calling the {@link #acquire()} method),
36-
* and is decremented (when a previously acquired segment is later closed).
38+
* This class manages the temporal bounds associated with a memory segment as well
39+
* as thread confinement.
40+
* A scope has a liveness bit, which is updated when the scope is closed
41+
* (this operation is triggered by {@link AbstractMemorySegmentImpl#close()}).
42+
* A scope may also have an associated "owner" thread that confines some operations to
43+
* associated owner thread such as {@link #close()} or {@link #dup(Thread)}.
44+
* Furthermore, a scope is either root scope ({@link #create(Object, Runnable) created}
45+
* when memory segment is allocated) or child scope ({@link #acquire() acquired} from root scope).
46+
* When a child scope is acquired from another child scope, it is actually acquired from
47+
* the root scope. There is only a single level of children. All child scopes are peers.
48+
* A child scope can be {@link #close() closed} at any time, but root scope can only
49+
* be closed after all its children have been closed, at which time any associated
50+
* cleanup action is executed (the associated memory segment is freed).
51+
* Besides thread-confined checked scopes, {@linkplain #createUnchecked(Thread, Object, Runnable)}
52+
* method may be used passing {@code null} as the "owner" thread to create a
53+
* scope that doesn't check for thread-confinement while its temporal bounds are
54+
* enforced reliably only under condition that thread that closes the scope is also
55+
* the single thread performing the checked access or there is an external synchronization
56+
* in place that prevents concurrent access and closing of the scope.
3757
*/
38-
public final class MemoryScope {
58+
abstract class MemoryScope {
3959

40-
//reference to keep hold onto
41-
final Object ref;
60+
/**
61+
* Creates a root MemoryScope with given ref, cleanupAction and current
62+
* thread as the "owner" thread.
63+
* This method may be called in any thread.
64+
* The returned instance may be published unsafely to and used in any thread,
65+
* but methods that explicitly state that they may only be called in "owner" thread,
66+
* must strictly be called in the thread that created the scope
67+
* or else IllegalStateException is thrown.
68+
*
69+
* @param ref an optional reference to an instance that needs to be kept reachable
70+
* @param cleanupAction an optional cleanup action to be executed when returned scope is closed
71+
* @return a root MemoryScope
72+
*/
73+
static MemoryScope create(Object ref, Runnable cleanupAction) {
74+
return new Root(Thread.currentThread(), ref, cleanupAction);
75+
}
4276

43-
int activeCount = UNACQUIRED;
77+
/**
78+
* Creates a root MemoryScope with given ref, cleanupAction and "owner" thread.
79+
* This method may be called in any thread.
80+
* The returned instance may be published unsafely to and used in any thread,
81+
* but methods that explicitly state that they may only be called in "owner" thread,
82+
* must strictly be called in given owner thread or else IllegalStateException is thrown.
83+
* If given owner thread is null, the returned MemoryScope is unchecked, meaning
84+
* that all methods may be called in any thread and that {@link #checkValidState()}
85+
* does not check for temporal bounds.
86+
*
87+
* @param owner the desired owner thread. If {@code owner == null},
88+
* the returned scope is <em>not</em> thread-confined and not checked.
89+
* @param ref an optional reference to an instance that needs to be kept reachable
90+
* @param cleanupAction an optional cleanup action to be executed when returned scope is closed
91+
* @return a root MemoryScope
92+
*/
93+
static MemoryScope createUnchecked(Thread owner, Object ref, Runnable cleanupAction) {
94+
return new Root(owner, ref, cleanupAction);
95+
}
4496

45-
final static VarHandle COUNT_HANDLE;
97+
private final Thread owner;
98+
private boolean closed; // = false
99+
private static final VarHandle CLOSED;
46100

47101
static {
48102
try {
49-
COUNT_HANDLE = MethodHandles.lookup().findVarHandle(MemoryScope.class, "activeCount", int.class);
103+
CLOSED = MethodHandles.lookup().findVarHandle(MemoryScope.class, "closed", boolean.class);
50104
} catch (Throwable ex) {
51105
throw new ExceptionInInitializerError(ex);
52106
}
53107
}
54108

55-
final static int UNACQUIRED = 0;
56-
final static int CLOSED = -1;
57-
final static int MAX_ACQUIRE = Integer.MAX_VALUE;
109+
private MemoryScope(Thread owner) {
110+
this.owner = owner;
111+
}
58112

59-
final Runnable cleanupAction;
113+
/**
114+
* Acquires a child scope (or peer scope if this is a child) with current
115+
* thread as the "owner" thread.
116+
* This method may be called in any thread.
117+
* The returned instance may be published unsafely to and used in any thread,
118+
* but methods that explicitly state that they may only be called in "owner" thread,
119+
* must strictly be called in the thread that acquired the scope
120+
* or else IllegalStateException is thrown.
121+
*
122+
* @return a child (or peer) scope
123+
* @throws IllegalStateException if root scope is already closed
124+
*/
125+
abstract MemoryScope acquire();
60126

61-
final static MemoryScope GLOBAL = new MemoryScope(null, null);
127+
/**
128+
* Closes this scope, executing any cleanup action if this is the root scope.
129+
* This method may only be called in the "owner" thread of this scope unless the
130+
* scope is a root scope with no owner thread - i.e. is not checked.
131+
*
132+
* @throws IllegalStateException if this scope is already closed or if this is
133+
* a root scope and there is/are still active child
134+
* scope(s) or if this method is called outside of
135+
* owner thread in checked scope
136+
*/
137+
abstract void close();
62138

63-
public MemoryScope(Object ref, Runnable cleanupAction) {
64-
this.ref = ref;
65-
this.cleanupAction = cleanupAction;
139+
/**
140+
* Duplicates this scope with given new "owner" thread and {@link #close() closes} it.
141+
* If this is a root scope, a new root scope is returned; this root scope is closed, but
142+
* without executing the cleanup action, which is instead transferred to the duped scope.
143+
* If this is a child scope, a new child scope is returned.
144+
* This method may only be called in the "owner" thread of this scope unless the
145+
* scope is a root scope with no owner thread - i.e. is not checked.
146+
* The returned instance may be published unsafely to and used in any thread,
147+
* but methods that explicitly state that they may only be called in "owner" thread,
148+
* must strictly be called in given new "owner" thread
149+
* or else IllegalStateException is thrown.
150+
*
151+
* @param newOwner new owner thread of the returned MemoryScope
152+
* @return a duplicate of this scope
153+
* @throws NullPointerException if given owner thread is null
154+
* @throws IllegalStateException if this scope is already closed or if this is
155+
* a root scope and there is/are still active child
156+
* scope(s) or if this method is called outside of
157+
* owner thread in checked scope
158+
*/
159+
abstract MemoryScope dup(Thread newOwner);
160+
161+
/**
162+
* Returns "owner" thread of this scope.
163+
*
164+
* @return owner thread (or null for unchecked scope)
165+
*/
166+
final Thread ownerThread() {
167+
return owner;
66168
}
67169

68170
/**
69-
* This method performs a full, thread-safe liveness check; can be used outside confinement thread.
171+
* This method may be called in any thread.
172+
*
173+
* @return {@code true} if this scope is not closed yet.
70174
*/
71-
final boolean isAliveThreadSafe() {
72-
return ((int)COUNT_HANDLE.getVolatile(this)) != CLOSED;
175+
final boolean isAlive() {
176+
return !((boolean)CLOSED.getVolatile(this));
73177
}
74178

75179
/**
76-
* This method performs a quick liveness check; must be called from the confinement thread.
180+
* Checks that this scope is still alive and that this method is executed
181+
* in the "owner" thread of this scope or this scope is unchecked (not associated
182+
* with owner thread).
183+
*
184+
* @throws IllegalStateException if this scope is already closed or this
185+
* method is executed outside owning thread
186+
* in checked scope
77187
*/
78-
final void checkAliveConfined() {
79-
if (activeCount == CLOSED) {
80-
throw new IllegalStateException("Segment is not alive");
188+
@ForceInline
189+
final void checkValidState() {
190+
if (owner != null && owner != Thread.currentThread()) {
191+
throw new IllegalStateException("Attempted access outside owning thread");
81192
}
193+
checkAliveConfined(this);
82194
}
83195

84-
MemoryScope acquire() {
85-
int value;
86-
do {
87-
value = (int)COUNT_HANDLE.getVolatile(this);
88-
if (value == CLOSED) {
89-
//segment is not alive!
90-
throw new IllegalStateException("Segment is not alive");
91-
} else if (value == MAX_ACQUIRE) {
92-
//overflow
93-
throw new IllegalStateException("Segment acquire limit exceeded");
94-
}
95-
} while (!COUNT_HANDLE.compareAndSet(this, value, value + 1));
96-
return new MemoryScope(ref, this::release);
196+
/**
197+
* Checks that this scope is still alive.
198+
*
199+
* @throws IllegalStateException if this scope is already closed
200+
*/
201+
@ForceInline
202+
private static void checkAliveConfined(MemoryScope scope) {
203+
if (scope.closed) {
204+
throw new IllegalStateException("This segment is already closed");
205+
}
97206
}
98207

99-
private void release() {
100-
int value;
101-
do {
102-
value = (int)COUNT_HANDLE.getVolatile(this);
103-
if (value <= UNACQUIRED) {
104-
//cannot get here - we can't close segment twice
105-
throw new IllegalStateException();
208+
private static final class Root extends MemoryScope {
209+
private final StampedLock lock = new StampedLock();
210+
private final LongAdder acquired = new LongAdder();
211+
private final Object ref;
212+
private final Runnable cleanupAction;
213+
214+
private Root(Thread owner, Object ref, Runnable cleanupAction) {
215+
super(owner);
216+
this.ref = ref;
217+
this.cleanupAction = cleanupAction;
218+
}
219+
220+
@Override
221+
MemoryScope acquire() {
222+
// try to optimistically acquire the lock
223+
long stamp = lock.tryOptimisticRead();
224+
try {
225+
for (; ; stamp = lock.readLock()) {
226+
if (stamp == 0L)
227+
continue;
228+
checkAliveConfined(this); // plain read is enough here (either successful optimistic read, or full read lock)
229+
230+
// increment acquires
231+
acquired.increment();
232+
// did a call to close() occur since we acquired the lock?
233+
if (lock.validate(stamp)) {
234+
// no, just return the acquired scope
235+
return new Child(Thread.currentThread());
236+
} else {
237+
// yes, just back off and retry (close might have failed, after all)
238+
acquired.decrement();
239+
}
240+
}
241+
} finally {
242+
if (StampedLock.isReadLockStamp(stamp))
243+
lock.unlockRead(stamp);
106244
}
107-
} while (!COUNT_HANDLE.compareAndSet(this, value, value - 1));
108-
}
245+
}
109246

110-
void close(boolean doCleanup) {
111-
if (!COUNT_HANDLE.compareAndSet(this, UNACQUIRED, CLOSED)) {
112-
//first check if already closed...
113-
checkAliveConfined();
114-
//...if not, then we have acquired views that are still active
115-
throw new IllegalStateException("Cannot close a segment that has active acquired views");
247+
@Override
248+
MemoryScope dup(Thread newOwner) {
249+
Objects.requireNonNull(newOwner, "newOwner");
250+
// pre-allocate duped scope so we don't get OOME later and be left with this scope closed
251+
var duped = new Root(newOwner, ref, cleanupAction);
252+
justClose();
253+
return duped;
116254
}
117-
if (doCleanup && cleanupAction != null) {
118-
cleanupAction.run();
255+
256+
@Override
257+
void close() {
258+
justClose();
259+
if (cleanupAction != null) {
260+
cleanupAction.run();
261+
}
119262
}
120-
}
121263

122-
MemoryScope dup() {
123-
close(false);
124-
return new MemoryScope(ref, cleanupAction);
264+
@ForceInline
265+
private void justClose() {
266+
// enter critical section - no acquires are possible past this point
267+
long stamp = lock.writeLock();
268+
try {
269+
checkValidState(); // plain read is enough here (full write lock)
270+
// check for absence of active acquired children
271+
if (acquired.sum() > 0) {
272+
throw new IllegalStateException("Cannot close this scope as it has active acquired children");
273+
}
274+
// now that we made sure there's no active acquired children, we can mark scope as closed
275+
CLOSED.set(this, true); // plain write is enough here (full write lock)
276+
} finally {
277+
// leave critical section
278+
lock.unlockWrite(stamp);
279+
}
280+
}
281+
282+
private final class Child extends MemoryScope {
283+
284+
private Child(Thread owner) {
285+
super(owner);
286+
}
287+
288+
@Override
289+
MemoryScope acquire() {
290+
return Root.this.acquire();
291+
}
292+
293+
@Override
294+
MemoryScope dup(Thread newOwner) {
295+
checkValidState(); // child scope is always checked
296+
// pre-allocate duped scope so we don't get OOME later and be left with this scope closed
297+
var duped = new Child(newOwner);
298+
CLOSED.setVolatile(this, true);
299+
return duped;
300+
}
301+
302+
@Override
303+
void close() {
304+
checkValidState(); // child scope is always checked
305+
CLOSED.set(this, true);
306+
// following acts as a volatile write after plain write above so
307+
// plain write gets flushed too (which is important for isAliveThreadSafe())
308+
Root.this.acquired.decrement();
309+
}
310+
}
125311
}
126312
}

‎src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/NativeMemorySegmentImpl.java

+9-9
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,14 @@ public class NativeMemorySegmentImpl extends AbstractMemorySegmentImpl {
5353
final long min;
5454

5555
@ForceInline
56-
NativeMemorySegmentImpl(long min, long length, int mask, Thread owner, MemoryScope scope) {
57-
super(length, mask, owner, scope);
56+
NativeMemorySegmentImpl(long min, long length, int mask, MemoryScope scope) {
57+
super(length, mask, scope);
5858
this.min = min;
5959
}
6060

6161
@Override
62-
NativeMemorySegmentImpl dup(long offset, long size, int mask, Thread owner, MemoryScope scope) {
63-
return new NativeMemorySegmentImpl(min + offset, size, mask, owner, scope);
62+
NativeMemorySegmentImpl dup(long offset, long size, int mask, MemoryScope scope) {
63+
return new NativeMemorySegmentImpl(min + offset, size, mask, scope);
6464
}
6565

6666
@Override
@@ -93,9 +93,9 @@ public static MemorySegment makeNativeSegment(long bytesSize, long alignmentByte
9393
unsafe.setMemory(buf, alignedSize, (byte)0);
9494
}
9595
long alignedBuf = Utils.alignUp(buf, alignmentBytes);
96-
MemoryScope scope = new MemoryScope(null, () -> unsafe.freeMemory(buf));
97-
MemorySegment segment = new NativeMemorySegmentImpl(buf, alignedSize, defaultAccessModes(alignedSize),
98-
Thread.currentThread(), scope);
96+
MemoryScope scope = MemoryScope.create(null, () -> unsafe.freeMemory(buf));
97+
MemorySegment segment = new NativeMemorySegmentImpl(buf, alignedSize,
98+
defaultAccessModes(alignedSize), scope);
9999
if (alignedSize != bytesSize) {
100100
long delta = alignedBuf - buf;
101101
segment = segment.asSlice(delta, bytesSize);
@@ -104,7 +104,7 @@ public static MemorySegment makeNativeSegment(long bytesSize, long alignmentByte
104104
}
105105

106106
public static MemorySegment makeNativeSegmentUnchecked(MemoryAddress min, long bytesSize, Thread owner, Runnable cleanup, Object attachment) {
107-
MemoryScope scope = new MemoryScope(attachment, cleanup);
108-
return new NativeMemorySegmentImpl(min.toRawLongValue(), bytesSize, defaultAccessModes(bytesSize), owner, scope);
107+
MemoryScope scope = MemoryScope.createUnchecked(owner, attachment, cleanup);
108+
return new NativeMemorySegmentImpl(min.toRawLongValue(), bytesSize, defaultAccessModes(bytesSize), scope);
109109
}
110110
}

‎test/jdk/java/foreign/TestByteBuffer.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ public void testScopedBuffer(Function<ByteBuffer, Buffer> bufferFactory, Map<Met
311311
Throwable cause = ex.getCause();
312312
if (cause instanceof IllegalStateException) {
313313
//all get/set buffer operation should fail because of the scope check
314-
assertTrue(ex.getCause().getMessage().contains("not alive"));
314+
assertTrue(ex.getCause().getMessage().contains("already closed"));
315315
} else {
316316
//all other exceptions were unexpected - fail
317317
assertTrue(false);
@@ -348,7 +348,7 @@ public void testScopedBufferAndVarHandle(VarHandle bufferHandle) {
348348
handle.invoke(e.getValue());
349349
fail();
350350
} catch (IllegalStateException ex) {
351-
assertTrue(ex.getMessage().contains("not alive"));
351+
assertTrue(ex.getMessage().contains("already closed"));
352352
} catch (UnsupportedOperationException ex) {
353353
//skip
354354
} catch (Throwable ex) {

0 commit comments

Comments
 (0)
Please sign in to comment.