On Mon, 7 Sep 2026 17:35:34 GMT, Per Minborg <[email protected]> wrote:
>> The methods `load()`, `unload()`, `isLoaded()`, and `force()` in >> `MemorySegment` currently delegate to `ScopedMemoryAccess` through a set of >> `@Scoped` methods, after which the implementation calls into >> `java.nio.MappedMemoryUtils`. This means that, when a shared scope is closed >> during a call to one of these methods, an exception can be installed at any >> point during the execution of the util method. >> >> The problem is that some parts of these methods are not able to handle such >> exceptions being installed. >> >> We've had some previous discussion about these methods not really needing to >> be `@Scoped` in the first place, but instead being able to rely on paired >> acquire/release of the session being accessed. This code is not as >> performance critical compared to a scoped memory access, since we're doing a >> native call any way. >> >> To avoid issues with exceptions being installed in surprising places, this >> patch switches the named methods to use acquire/release instead of being >> `@Scoped`. This changes the behavior of these methods slightly: they now >> keep the scope alive during the execution of the method. I've updated the >> doc, borrowing from existing text in the `Linked::downcallHandle` docs, to >> explain that a scope closure may now fail during the execution of one of >> these methods. >> >> Does this seem like the right tradeoff? >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > src/java.base/share/classes/java/lang/foreign/Linker.java line 635: > >> 633: * {@code A.isAccessibleBy(T) == true}. >> 634: * Otherwise, the invocation throws {@link >> WrongThreadException}; and</li> >> 635: * <li>{@code A} is {@linkplain MemorySegment##keep-alive kept >> alive} > > I am not an expert here, but would it make sense to say this only applies if > there is a call by reference (i.e., an `AddressLayout` is involved)? This text only applies to arguments with an `AddressLayout`. See the text before this list: Upon invoking a downcall method handle, the linker provides the following * guarantees for any argument {@code A} of type {@link MemorySegment} whose * corresponding layout is an {@linkplain AddressLayout address layout}: > src/java.base/share/classes/jdk/internal/misc/X-ScopedMemoryAccess.java.template > line 244: > >> 242: try { >> 243: if (session != null) { >> 244: session.acquire0(); > > Could we move `acquire0()` before the TWR-block, or otherwise ensure that > `release0()` is called only after a successful acquisition? As written, if > `acquire0()` fails (e.g., because of `WrongThreadException`), the bookkeeping > gets wrong with unmatched accountings. > > Perhaps we should add a regression test for this as well? Good catch! We should indeed move the acquire outside of the try block. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31918#discussion_r3951760111 PR Review Comment: https://git.openjdk.org/jdk/pull/31918#discussion_r3951767102
