Inappropriate use of the implicit session in allocating the upcall stub in SafeFunctionAccessTest.java

[Jorn Vernee]

Hi Cheng Jin,

Thanks for the email.

In this case it is not a problem since memory sessions (including upcall 
stubs) are kept alive/reachable for the duration of a downcall (see the 
third bullet here: [1])

     "The memory session of R is kept alive (and cannot be closed) 
during the invocation."

Since we don't store a reference to the upcall stub in native code until 
after the downcall returns, this is safe to rely on.

Cheers,
Jorn

[1]: 
https://docs.oracle.com/en/java/javase/19/docs/api/java.base/java/lang/foreign/Linker.html#safety

On 31/01/2023 18:49, Cheng Jin wrote:
>
> Hi there,
>
> I notice there might be an issue with the allocation of upcall stub 
> under an implicit session at 
> test/jdk/java/foreign/SafeFunctionAccessTest.java in JDK19+ as follows:
>
> e.g.
>
>     @Test
>
>     public void testClosedStructCallback() throws Throwable {
>
>         MethodHandle handle = Linker.nativeLinker().downcallHandle(
>
> findNativeOrThrow("addr_func_cb"),
>
> FunctionDescriptor.ofVoid(C_POINTER, C_POINTER));
>
>         try (MemorySession session = MemorySession.openConfined()) {
>
>            MemorySegment segment = MemorySegment.allocateNative(POINT, 
> session);
>
>             handle.invoke(segment, sessionChecker(session)); 
> ß--------------- the upcall stub is allocated in sessionChecker()
>
>         }
>
>     }
>
>     MemorySegment sessionChecker(MemorySession session) {
>
>         try {
>
>             MethodHandle handle = 
> MethodHandles.lookup().findStatic(SafeFunctionAccessTest.class, 
> "checkSession",
>
> MethodType.methodType(void.class, MemorySession.class));
>
>             handle = handle.bindTo(session);
>
>             return Linker.nativeLinker().upcallStub(handle, 
> FunctionDescriptor.ofVoid(), MemorySession.openImplicit()); ß-----the 
> upcall stub is allocated with an implicit session
>
>         } catch (Throwable ex) {
>
>             throw new AssertionError(ex);
>
>         }
>
> }
>
> It is correct to allocate the upcall stub with a session different 
> from the current session in tests given these tests intend to close 
> the current session in upcall to verify the behavior. But it is 
> problematic to exploit an implicit session backed by GC, in which case 
> the memory of the upcall stub is more likely to be forced to release 
> by GC (when implicitly closing the session to free memory) especially 
> on the memory-restricted machines, which leads to unexpected behavior 
> in upcall (e.g. crash which was captured recently). To work around 
> this case, I’d suggest to replace it with a global session or others 
> so as to keep the upcall stub alive during the upcall.
>
> Best Regards
>
> Cheng Jin
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/panama-dev/attachments/20230131/0f63f9bc/attachment-0001.htm>