Can access closed memory segment

[forax at univ-mlv.fr]

> From: "Maurizio Cimadamore" <maurizio.cimadamore at oracle.com>
> To: "Remi Forax" <forax at univ-mlv.fr>, "panama-dev" <panama-dev at openjdk.org>
> Sent: Tuesday, June 18, 2024 11:28:59 AM
> Subject: Re: Can access closed memory segment

> 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.

> 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.

> As an experiment, I made STATE volatile, and now the program reliably crashes
> with this:

> Exception in thread “Thread-0” java.lang.IllegalStateException: Already closed
> at
> java.base/jdk.internal.foreign.MemorySessionImpl.alreadyClosed(MemorySessionImpl.java:310)
> at
> java.base/jdk.internal.misc.ScopedMemoryAccess$ScopedAccessError.newRuntimeException(ScopedMemoryAccess.java:113)
> at
> java.base/jdk.internal.misc.ScopedMemoryAccess.getByte(ScopedMemoryAccess.java:502)
> at
> java.base/java.lang.invoke.VarHandleSegmentAsBytes.get(VarHandleSegmentAsBytes.java:108)
> at
> java.base/jdk.internal.foreign.AbstractMemorySegmentImpl.get(AbstractMemorySegmentImpl.java:728)
> at SharedArenaCloseBug.run(SharedArenaCloseBug.java:11)
> at java.base/java.lang.Thread.run(Thread.java:1570)

> Which seems correct for a use-after-free.

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. 

> I also noticed that STATE = 1 in the closing thread is set after starting the
> accessing thread.

> So, a possible execution order here is one where:

>    * the accessing thread doesn’t see the update to STATE = 1 at all (because,
>     plain read)
>     * the closing thread writes 42
>     * the accessing thread sees 42 (but still sees STATE = 0)
>     * the accessing thread completes
>     * the closing thread closes the segment

> 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).

yes, this is a possible ordering. 

> As further test, I’ve tweaked the accessing thread like this:
> if (STATE == 0 ) { ... } else {
>      System.out.println( "NOPE" );
> }

> And verified that “NOPE” is never printed. So the accessing thread doesn’t
> really seem to ever see STATE != 0.

yes, you are right, so the VM now optimizes the static non-final read, something that was not done in the past, 
so yes, my test is bogus, i'm less worry now. 

> Maurizio

Rémi 

> On 18/06/2024 08:54, Remi Forax wrote:

>> 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

> ​
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/panama-dev/attachments/20240618/1f5b2011/attachment-0001.htm>