<html><body><div style="font-family: arial, helvetica, sans-serif; font-size: 12pt; color: #000000"><div><br></div><div><br></div><hr id="zwchr" data-marker="__DIVIDER__"><div data-marker="__HEADERS__"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><b>From: </b>"Maurizio Cimadamore" <maurizio.cimadamore@oracle.com><br><b>To: </b>"Remi Forax" <forax@univ-mlv.fr>, "panama-dev" <panama-dev@openjdk.org><br><b>Sent: </b>Tuesday, June 18, 2024 11:28:59 AM<br><b>Subject: </b>Re: Can access closed memory segment<br></blockquote></div><div data-marker="__QUOTED_TEXT__"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div class="markdown-here-wrapper" style="">
<p style="margin: 0px 0px 1.2em !important;">So… I have some
questions on the test… I ran it here and all looks fine (e.g. no
crash), which is I think what you are assuming to take as
evidence of a bug.</p>
<p style="margin: 0px 0px 1.2em !important;">But… the update to
the STATE variable is plain. E.g. STATE is not a volatile
variable and yet you are using it to synchronize work across the
closing/accessing threads.</p>
<p style="margin: 0px 0px 1.2em !important;">As an experiment, I
made STATE volatile, and now the program reliably crashes with
this:</p>
<p style="margin: 0px 0px 1.2em !important;">Exception in thread
“Thread-0” java.lang.IllegalStateException: Already closed<br>
at
java.base/jdk.internal.foreign.MemorySessionImpl.alreadyClosed(MemorySessionImpl.java:310)<br>
at
java.base/jdk.internal.misc.ScopedMemoryAccess$ScopedAccessError.newRuntimeException(ScopedMemoryAccess.java:113)<br>
at
java.base/jdk.internal.misc.ScopedMemoryAccess.getByte(ScopedMemoryAccess.java:502)<br>
at
java.base/java.lang.invoke.VarHandleSegmentAsBytes.get(VarHandleSegmentAsBytes.java:108)<br>
at
java.base/jdk.internal.foreign.AbstractMemorySegmentImpl.get(AbstractMemorySegmentImpl.java:728)<br>
at SharedArenaCloseBug.run(SharedArenaCloseBug.java:11)<br>
at java.base/java.lang.Thread.run(Thread.java:1570)</p>
<p style="margin: 0px 0px 1.2em !important;">Which seems correct
for a use-after-free.</p></div></blockquote><div><br></div><div>The problem of volatile is that it has a broad effect, reading a volatile forces all next read to be read from the RAM, so here it can be the effect of volatile.<br data-mce-bogus="1"></div><div><br data-mce-bogus="1"></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div class="markdown-here-wrapper" style="">
<p style="margin: 0px 0px 1.2em !important;">I also noticed that
STATE = 1 in the closing thread is set <em>after</em> starting
the accessing thread. </p>
<p style="margin: 0px 0px 1.2em !important;">So, a possible
execution order here is one where:</p>
<ul style="margin: 1.2em 0px;padding-left: 2em;">
<li style="margin: 0.5em 0px;">the accessing thread doesn’t see
the update to STATE = 1 at all (because, plain read)</li>
<li style="margin: 0.5em 0px;">the closing thread writes 42</li>
<li style="margin: 0.5em 0px;">the accessing thread sees 42 (but
still sees STATE = 0)</li>
<li style="margin: 0.5em 0px;">the accessing thread completes</li>
<li style="margin: 0.5em 0px;">the closing thread closes the
segment</li>
</ul>
<p style="margin: 0px 0px 1.2em !important;">Since your test rely
on the fact that STATE should be “1” when value is written, and
the segment is closed, I think a plain write/read is not
sufficient to establish this kind of ordering between the two
threads (and that would explain while adding a “volatile” on
STATE “fixes” things).</p></div></blockquote><div><br></div><div>yes, this is a possible ordering.<br data-mce-bogus="1"></div><div><br data-mce-bogus="1"></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div class="markdown-here-wrapper" style="">
<p style="margin: 0px 0px 1.2em !important;">As further test, I’ve
tweaked the accessing thread like this:</p>
<pre style="font-size: 0.85em; font-family: Consolas, Inconsolata, Courier, monospace;font-size: 1em; line-height: 1.2em;margin: 1.2em 0px;"><code class="hljs language-java" style="font-size: 0.85em; font-family: Consolas, Inconsolata, Courier, monospace;margin: 0px 0.15em; padding: 0px 0.3em; white-space: pre-wrap; border: 1px solid rgb(234, 234, 234); background-color: rgb(248, 248, 248); border-radius: 3px; display: inline;white-space: pre; overflow: auto; border-radius: 3px; border: 1px solid rgb(204, 204, 204); padding: 0.5em 0.7em; display: block !important;display: block; overflow-x: auto; padding: 0.5em; color: rgb(51, 51, 51); background: rgb(248, 248, 248); -moz-text-size-adjust: none;"><span class="hljs-keyword" style="color: rgb(51, 51, 51); font-weight: bold;">if</span> (STATE == <span class="hljs-number" style="color: rgb(0, 128, 128);">0</span>) { ... }
<span class="hljs-keyword" style="color: rgb(51, 51, 51); font-weight: bold;">else</span> {
System.out.println(<span class="hljs-string" style="color: rgb(221, 17, 68);">"NOPE"</span>);
}
</code></pre>
<p style="margin: 0px 0px 1.2em !important;">And verified that
“NOPE” is never printed. So the accessing thread doesn’t really
seem to ever see STATE != 0.</p></div></blockquote><div><br data-mce-bogus="1"></div><div>yes, you are right, so the VM now optimizes the static non-final read, something that was not done in the past,<br data-mce-bogus="1"></div><div>so yes, my test is bogus, i'm less worry now.<br data-mce-bogus="1"></div><div><br data-mce-bogus="1"></div><div><br data-mce-bogus="1"></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div class="markdown-here-wrapper" style="">
<p style="margin: 0px 0px 1.2em !important;">Maurizio</p></div></blockquote><div><br></div><div>Rémi<br data-mce-bogus="1"></div><div><br data-mce-bogus="1"></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div class="markdown-here-wrapper" style="">
<p style="margin: 0px 0px 1.2em !important;">On 18/06/2024 08:54,
Remi Forax wrote:</p>
<div class="markdown-here-exclude">
<blockquote cite="mid:1635633843.52889282.1718697275443.JavaMail.zimbra@univ-eiffel.fr">
<pre class="moz-quote-pre">Hello,
I think i've found an issue in the current implementation of shared arena close(),
when closing the scope the VM only checks if the arena is on stack but the memory segment can be stored in a static final.
In the following code, i'm able to read the segment after the shared arena is closed.
public class SharedArenaCloseBug {
static final Arena SHARED = Arena.ofShared();
static final MemorySegment SEGMENT = SHARED.allocate(16);
static int STATE;
void run() {
for(;;) {
if (STATE == 0) {
var value = SEGMENT.get(ValueLayout.JAVA_BYTE, 0L);
if (value == 42) {
return;
}
}
}
}
public static void main(String[] args) throws InterruptedException {
var bug = new SharedArenaCloseBug();
var thread = new Thread(bug::run);
thread.start();
Thread.sleep(100);
STATE = 1;
System.out.println("state 1");
Thread.sleep(100);
System.out.println("close");
SEGMENT.set(ValueLayout.JAVA_BYTE, 0L, (byte) 42);
SHARED.close();
Thread.sleep(100);
System.out.println("state 0");
STATE = 0;
Thread.sleep(100);
System.out.println("join");
thread.join();
}
}
regards,
Rémi
</pre>
</blockquote>
</div>
<div title="MDH:PHA+U28uLi4gSSBoYXZlIHNvbWUgcXVlc3Rpb25zIG9uIHRoZSB0ZXN0Li4uIEkgcmFuIGl0IGhl
cmUgYW5kIGFsbCBsb29rcyBmaW5lIChlLmcuIG5vIGNyYXNoKSwgd2hpY2ggaXMgSSB0aGluayB3
aGF0IHlvdSBhcmUgYXNzdW1pbmcgdG8gdGFrZSBhcyBldmlkZW5jZSBvZiBhIGJ1Zy48L3A+PHA+
QnV0Li4uIHRoZSB1cGRhdGUgdG8gdGhlIFNUQVRFIHZhcmlhYmxlIGlzIHBsYWluLiBFLmcuIFNU
QVRFIGlzIG5vdCBhIHZvbGF0aWxlIHZhcmlhYmxlIGFuZCB5ZXQgeW91IGFyZSB1c2luZyBpdCB0
byBzeW5jaHJvbml6ZSB3b3JrIGFjcm9zcyB0aGUgY2xvc2luZy9hY2Nlc3NpbmcgdGhyZWFkcy48
L3A+PHA+QXMgYW4gZXhwZXJpbWVudCwgSSBtYWRlIFNUQVRFIHZvbGF0aWxlLCBhbmQgbm93IHRo
ZSBwcm9ncmFtIHJlbGlhYmx5IGNyYXNoZXMgd2l0aCB0aGlzOjwvcD48cD5FeGNlcHRpb24gaW4g
dGhyZWFkICJUaHJlYWQtMCIgamF2YS5sYW5nLklsbGVnYWxTdGF0ZUV4Y2VwdGlvbjogQWxyZWFk
eSBjbG9zZWQ8YnI+Jm5ic3A7Jm5ic3A7ICZuYnNwO2F0IGphdmEuYmFzZS9qZGsuaW50ZXJuYWwu
Zm9yZWlnbi5NZW1vcnlTZXNzaW9uSW1wbC5hbHJlYWR5Q2xvc2VkKE1lbW9yeVNlc3Npb25JbXBs
LmphdmE6MzEwKTxicj4mbmJzcDsmbmJzcDsgJm5ic3A7YXQgamF2YS5iYXNlL2pkay5pbnRlcm5h
bC5taXNjLlNjb3BlZE1lbW9yeUFjY2VzcyRTY29wZWRBY2Nlc3NFcnJvci5uZXdSdW50aW1lRXhj
ZXB0aW9uKFNjb3BlZE1lbW9yeUFjY2Vzcy5qYXZhOjExMyk8YnI+Jm5ic3A7Jm5ic3A7ICZuYnNw
O2F0IGphdmEuYmFzZS9qZGsuaW50ZXJuYWwubWlzYy5TY29wZWRNZW1vcnlBY2Nlc3MuZ2V0Qnl0
ZShTY29wZWRNZW1vcnlBY2Nlc3MuamF2YTo1MDIpPGJyPiZuYnNwOyZuYnNwOyAmbmJzcDthdCBq
YXZhLmJhc2UvamF2YS5sYW5nLmludm9rZS5WYXJIYW5kbGVTZWdtZW50QXNCeXRlcy5nZXQoVmFy
SGFuZGxlU2VnbWVudEFzQnl0ZXMuamF2YToxMDgpPGJyPiZuYnNwOyZuYnNwOyAmbmJzcDthdCBq
YXZhLmJhc2UvamRrLmludGVybmFsLmZvcmVpZ24uQWJzdHJhY3RNZW1vcnlTZWdtZW50SW1wbC5n
ZXQoQWJzdHJhY3RNZW1vcnlTZWdtZW50SW1wbC5qYXZhOjcyOCk8YnI+Jm5ic3A7Jm5ic3A7ICZu
YnNwO2F0IFNoYXJlZEFyZW5hQ2xvc2VCdWcucnVuKFNoYXJlZEFyZW5hQ2xvc2VCdWcuamF2YTox
MSk8YnI+Jm5ic3A7Jm5ic3A7ICZuYnNwO2F0IGphdmEuYmFzZS9qYXZhLmxhbmcuVGhyZWFkLnJ1
bihUaHJlYWQuamF2YToxNTcwKTxicj48YnI+V2hpY2ggc2VlbXMgY29ycmVjdCBmb3IgYSB1c2Ut
YWZ0ZXItZnJlZS48YnI+PC9wPkkgYWxzbyBub3RpY2VkIHRoYXQgU1RBVEUgPSAxIGluIHRoZSBj
bG9zaW5nIHRocmVhZCBpcyBzZXQgX2FmdGVyXyBzdGFydGluZyB0aGUgYWNjZXNzaW5nIHRocmVh
ZC4gPGJyPjxwPlNvLCBhIHBvc3NpYmxlIGV4ZWN1dGlvbiBvcmRlciBoZXJlIGlzIG9uZSB3aGVy
ZTo8YnI+IDxicj4qIHRoZSBhY2Nlc3NpbmcgdGhyZWFkIGRvZXNuJ3Qgc2VlIHRoZSB1cGRhdGUg
dG8gU1RBVEUgPSAxIGF0IGFsbCAoYmVjYXVzZSwgcGxhaW4gcmVhZCk8YnI+KiB0aGUgY2xvc2lu
ZyB0aHJlYWQgd3JpdGVzIDQyPGJyPiogdGhlIGFjY2Vzc2luZyB0aHJlYWQgc2VlcyA0MiAoYnV0
IHN0aWxsIHNlZXMgU1RBVEUgPSAwKTxicj4qIHRoZSBhY2Nlc3NpbmcgdGhyZWFkIGNvbXBsZXRl
czxicj4qIHRoZSBjbG9zaW5nIHRocmVhZCBjbG9zZXMgdGhlIHNlZ21lbnQ8YnI+PC9wPjxwPlNp
bmNlIHlvdXIgdGVzdCByZWx5IG9uIHRoZSBmYWN0IHRoYXQgU1RBVEUgc2hvdWxkIGJlICIxIiB3
aGVuIHZhbHVlIGlzIHdyaXR0ZW4sIGFuZCB0aGUgc2VnbWVudCBpcyBjbG9zZWQsIEkgdGhpbmsg
YSBwbGFpbiB3cml0ZS9yZWFkIGlzIG5vdCBzdWZmaWNpZW50IHRvIGVzdGFibGlzaCB0aGlzIGtp
bmQgb2Ygb3JkZXJpbmcgYmV0d2VlbiB0aGUgdHdvIHRocmVhZHMgKGFuZCB0aGF0IHdvdWxkIGV4
cGxhaW4gd2hpbGUgYWRkaW5nIGEgInZvbGF0aWxlIiBvbiBTVEFURSAiZml4ZXMiIHRoaW5ncyku
PC9wPjxwPkFzIGZ1cnRoZXIgdGVzdCwgSSd2ZSB0d2Vha2VkIHRoZSBhY2Nlc3NpbmcgdGhyZWFk
IGxpa2UgdGhpczo8L3A+PHA+YGBgamF2YTxicj5pZiAoU1RBVEUgPT0gMCkgeyAuLi4gfTxicj5l
bHNlIHs8YnI+Jm5ic3A7Jm5ic3A7Jm5ic3A7Jm5ic3A7IFN5c3RlbS5vdXQucHJpbnRsbigiTk9Q
RSIpOzxicj59PGJyPmBgYDwvcD48cD5BbmQgdmVyaWZpZWQgdGhhdCAiTk9QRSIgaXMgbmV2ZXIg
cHJpbnRlZC4gU28gdGhlIGFjY2Vzc2luZyB0aHJlYWQgZG9lc24ndCByZWFsbHkgc2VlbSB0byBl
dmVyIHNlZSBTVEFURSAhPSAwLjxicj48L3A+PHA+TWF1cml6aW88YnI+PC9wPjxwPjxicj48L3A+
PHA+T24gMTgvMDYvMjAyNCAwODo1NCwgUmVtaSBGb3JheCB3cm90ZTo8L3A+PGJsb2NrcXVvdGUg
dHlwZT0iY2l0ZSIgY2l0ZT0ibWlkOjE2MzU2MzM4NDMuNTI4ODkyODIuMTcxODY5NzI3NTQ0My5K
YXZhTWFpbC56aW1icmFAdW5pdi1laWZmZWwuZnIiPjxwcmUgY2xhc3M9Im1vei1xdW90ZS1wcmUi
IHdyYXA9IiI+SGVsbG8sCkkgdGhpbmsgaSd2ZSBmb3VuZCBhbiBpc3N1ZSBpbiB0aGUgY3VycmVu
dCBpbXBsZW1lbnRhdGlvbiBvZiBzaGFyZWQgYXJlbmEgY2xvc2UoKSwKd2hlbiBjbG9zaW5nIHRo
ZSBzY29wZSB0aGUgVk0gb25seSBjaGVja3MgaWYgdGhlIGFyZW5hIGlzIG9uIHN0YWNrIGJ1dCB0
aGUgbWVtb3J5IHNlZ21lbnQgY2FuIGJlIHN0b3JlZCBpbiBhIHN0YXRpYyBmaW5hbC4KCkluIHRo
ZSBmb2xsb3dpbmcgY29kZSwgaSdtIGFibGUgdG8gcmVhZCB0aGUgc2VnbWVudCBhZnRlciB0aGUg
c2hhcmVkIGFyZW5hIGlzIGNsb3NlZC4KCnB1YmxpYyBjbGFzcyBTaGFyZWRBcmVuYUNsb3NlQnVn
IHsKICBzdGF0aWMgZmluYWwgQXJlbmEgU0hBUkVEID0gQXJlbmEub2ZTaGFyZWQoKTsKICBzdGF0
aWMgZmluYWwgTWVtb3J5U2VnbWVudCBTRUdNRU5UID0gU0hBUkVELmFsbG9jYXRlKDE2KTsKICBz
dGF0aWMgaW50IFNUQVRFOwoKICB2b2lkIHJ1bigpIHsKICAgIGZvcig7OykgewogICAgICBpZiAo
U1RBVEUgPT0gMCkgewogICAgICAgIHZhciB2YWx1ZSA9IFNFR01FTlQuZ2V0KFZhbHVlTGF5b3V0
LkpBVkFfQllURSwgMEwpOwogICAgICAgIGlmICh2YWx1ZSA9PSA0MikgewogICAgICAgICAgcmV0
dXJuOwogICAgICAgIH0KICAgICAgfQogICAgfQogIH0KCiAgcHVibGljIHN0YXRpYyB2b2lkIG1h
aW4oU3RyaW5nW10gYXJncykgdGhyb3dzIEludGVycnVwdGVkRXhjZXB0aW9uIHsKICAgIHZhciBi
dWcgPSBuZXcgU2hhcmVkQXJlbmFDbG9zZUJ1ZygpOwogICAgdmFyIHRocmVhZCA9IG5ldyBUaHJl
YWQoYnVnOjpydW4pOwogICAgdGhyZWFkLnN0YXJ0KCk7CiAgICBUaHJlYWQuc2xlZXAoMTAwKTsK
ICAgIFNUQVRFID0gMTsKICAgIFN5c3RlbS5vdXQucHJpbnRsbigic3RhdGUgMSIpOwogICAgVGhy
ZWFkLnNsZWVwKDEwMCk7CiAgICBTeXN0ZW0ub3V0LnByaW50bG4oImNsb3NlIik7CiAgICBTRUdN
RU5ULnNldChWYWx1ZUxheW91dC5KQVZBX0JZVEUsIDBMLCAoYnl0ZSkgNDIpOwogICAgU0hBUkVE
LmNsb3NlKCk7CiAgICBUaHJlYWQuc2xlZXAoMTAwKTsKICAgIFN5c3RlbS5vdXQucHJpbnRsbigi
c3RhdGUgMCIpOwogICAgU1RBVEUgPSAwOwogICAgVGhyZWFkLnNsZWVwKDEwMCk7CiAgICBTeXN0
ZW0ub3V0LnByaW50bG4oImpvaW4iKTsKICAgIHRocmVhZC5qb2luKCk7CiAgfQp9CgpyZWdhcmRz
LApSw6ltaQo8L3ByZT48L2Jsb2NrcXVvdGU+" style="height:0;width:0;max-height:0;max-width:0;overflow:hidden;font-size:0em;padding:0;margin:0;"></div>
</div><br></blockquote></div></div></body></html>