MemorySession cleanup order

[Maurizio Cimadamore]

On 15/09/2022 18:06, Manuel Bleichenbacher wrote:
> Thank you for the detailed answer.
>
> It's quite confusing that the segment can still be copied but not used 
> in a method call. To understand it, it either requires detailed 
> knowledge about the implementation or a new concept or state related 
> to session and segments must documented.

Note that by "copied" we're probably referring to different things.

What I meant by "copying" was doing this:

```
assertFalse(oldSegment.session().isAlive());
MemorySegment newSegment = MemorySegment.ofAddress(oldSegment.address(), 
oldSegment.byteSize(), globalSession());
// now I can use newSegment!
```

The `ofAddress` factory creates a new native segment (unsafely), with a 
given base address, size and session. It does not copy any contents, it 
just assumes that the base address you provide is correct.


>
> Anyway, this would be even more cumbersome. So I'm sticking to other 
> approaches for cleanup.

One approach worth considering could be to also add an `addCloseAction` 
method on `MemorySegment` which basically is syntactic sugar for the 
code above:

```
void addCloseAction(Consumer<MemorySegment> action) {

     Runnable action = () -> {
         try (MemorySession closingSession = MemorySession.openConfined()) {
            MemorySegment dup = MemorySegment.ofAddress(this.address(), 
this.byteSize(), closingSession);
            action.accept(dup);
         }
     }
     session.addCloseAction(action);
}

```

If you have this method, then you can just do:

```
MemorySegment segment = MemorySegment.allocateNative(100, ...);
segment.addCloseAction( segment -> <insert logic here>);
```

Note that the segment the lambda operates on is not exactly the same 
segment as the original (it has a different memory session), but it is 
backed by the same memory region. Since we can guarantee that this 
custom cleanup action will always run _before_ the "free"/"unmap" 
operation (even in the case of multple threads), this would effectively 
solve some of the issues you brought up. Is that something that would be 
helpful? Perhaps you could try adding a method like the above in your 
code, but as a static method like this (since the implementation just 
relies on public API):

```

static void addCloseAction(MemorySegment segment, Consumer<MemorySegment> action) {
     long size = segment.byteSize();
     long address = segment.address();// or use MemoryAddress if on Java 19 Runnable closeAction = () -> {
         try (MemorySession closingSession = MemorySession.openConfined()) {
             MemorySegment dup = MemorySegment.ofAddress(address,size, closingSession);
             action.accept(dup);
         }
     };
     segment.session().addCloseAction(closeAction);
}

```

And maybe you can let us know how that goes?

Thanks
Maurizio



>
>
> On Wed, Sep 14, 2022 at 11:59 PM Maurizio Cimadamore 
> <maurizio.cimadamore at oracle.com> wrote:
>
>
>     On 14/09/2022 22:28, Manuel Bleichenbacher wrote:
>     > The documentation for MemorySession.addCloseAction() states that
>     the
>     > order of custom cleanup actions is unspecified. But it only
>     hints at
>     > the order of custom close actions vs. closing memory segments.
>     >
>     > Is it correct that memory segments are closed first, and then the
>     > custom cleanup actions are executed?
>     >
>     > If so, is there a specific reason for it? It would be more
>     useful to
>     > other way round.
>     >
>     > I have several cases requiring cleanup of data structures
>     residing in
>     > a memory segment. It would be most natural to use custom cleanup
>     > actions to do so as their lifespans end at the same time. But given
>     > the current order, a far less elegant way is needed.
>
>     The current spec says no ordering.
>
>     In reality there is an ordering that can be relied upon (the javadoc
>     will likley be rectified to reflect this).
>
>     The actions added to the scope last will also be called first. Of
>     course
>     the ordering is only valid for actions added within the same
>     thread, and
>     if you have multiple threads adding action, other orders could be
>     observed.
>
>     When you allocate a memory segment using a session, a cleanup
>     action for
>     it is added to the session (as if calling addCloseAction), so you
>     need
>     to take that into account as well.
>
>     Given all this, if you do:
>
>     ```
>     MemorySegment.allocateNative(100, session);
>     session.addCloseAction(runnable);
>     ```
>
>     I would expect the "runnable" to be executed before "free" is
>     called on
>     the memory segment.
>
>     But, mind you, that alone won't help much: from the perspective of
>     the
>     close action, the session attached to the segment has already been
>     closed, so you cannot touch the segment directly (by the same
>     token that
>     protects from use after free). In other words, a session is a bit of
>     state that is shared by all the resources attached to that
>     session. When
>     the session is closed (using the close() method), all the resources
>     attached to that session becomes inaccessible at once. There's no
>     way to
>     add a "pre-close" action, because, if the action is pre-close, the
>     session is still alive, and it means that other threads have
>     potentially
>     still access on the segment, and they might not know the segment is
>     about to be closed (so the close action would race with other
>     accesses,
>     which seems a recipe for disaster).
>
>     But the cleanup action can create a copy of the segment into a fresh
>     segment associated with the global scope (using
>     MemorySegment::ofAddress), and access that instead (since it knows
>     the
>     address still valid).
>
>     Maurizio
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/panama-dev/attachments/20220915/ebaf10d0/attachment-0001.htm>