[jdk17] RFR: 8269240: java/foreign/stackwalk/TestAsyncStackWalk.java test failed with concurrent GC

[Jorn Vernee]

On Fri, 25 Jun 2021 17:38:32 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> This patch rewrites the prologue and epilogue of panama upcalls, in order to fix the test failure from the title.
> 
> Previously, we did a call to potentially attach the current thread to the VM, and then afterwards did the same suspend and stack reguard checks that we do on the back-edge of a native downcall. Then, on the back edge of the upcall we did another conditional call to detach the thread.
> 
> I've changed these 2 calls to mimic what is done by JavaCallWrapper instead (with attach and detach included), and removed the old suspend and stack reguard checks (now handled by the call).
> 
> FWIW, this removes the JavaFrameAnchor save/restore MacroAssembler code. This is now written in C++. Also, MacroAssembler code was added to save/restore the result of the upcall around the call on the back-edge, which was previously missing. Since the new code allocates a handle block as well, I've added handling for those oops to frame & OptimizedUpcallBlob.
> 
> Testing: local running of `jdk_foreign` on Windows and Linux (WSL). Tier 1-3

src/hotspot/cpu/arm/frame_arm.cpp line 320:

> 318:   return nullptr;
> 319: }
> 320: 

FWIW, stubs were missing on some platforms, and this seems to have been fine before, but now started causing compilation errors, so I've added them here.

src/hotspot/share/runtime/safepoint.cpp line 931:

> 929:     // See if return type is an oop.
> 930:     bool return_oop = nm->method()->is_returning_oop();
> 931:     HandleMark hm(self);

I was seeing an `assert(_handle_mark_nesting > 1) failed: memory leak: allocating handle outside HandleMark` when the existing code allocates the Handle for `return_value` in the code down below. It seems to be a simple case of a missing handle mark, so I've added it here. (looking at the stack trace in the error log, the caller frame is native code, so I don't think we can expect the caller to have a HandleMark).

src/hotspot/share/runtime/thread.cpp line 1974:

> 1972:   assert(deferred_card_mark().is_empty(), "Should be empty during GC");
> 1973: 
> 1974:   // Traverse the GCHandles

I reduced some duplication here while debugging, but this change is not strictly needed (though a nice cleanup, IMO). Let me know, and I could remove this part if preferred.

test/jdk/java/foreign/stackwalk/TestAsyncStackWalk.java line 78:

> 76:  *   TestAsyncStackWalk
> 77:  */
> 78:  

Suggestion:

test/jdk/java/foreign/stackwalk/TestStackWalk.java line 78:

> 76:  *   TestStackWalk
> 77:  */
> 78:  

Suggestion:

-------------

PR: https://git.openjdk.java.net/jdk17/pull/149