On 11/11/2020 9:19 pm, Jorn Vernee wrote:
On Wed, 11 Nov 2020 07:18:33 GMT, David Holmes <dhol...@openjdk.org> wrote:

Maurizio Cimadamore has updated the pull request incrementally with 10 
additional commits since the last revision:

  - Merge pull request #7 from JornVernee/Additional_Review_Comments
Additional review comments
  - Revert System.java changes
  - Set copyright year for added files to 2020
  - Check result of AttachCurrentThread
  - Sort includes alphabetically
  - Relax ret_addr_offset() assert
  - Extra space after if
  - remove excessive asserts in ProgrammableInvoker::invoke_native
  - Remove os::is_MP() check
  - remove blank line in thread.hpp

src/hotspot/share/prims/universalUpcallHandler.cpp line 55:

53:     JavaVM_ *vm = (JavaVM *)(&main_vm);
54:     jint result = vm->functions->AttachCurrentThread(vm, (void**) &p_env, 
nullptr);
55:     guarantee(result == JNI_OK, "Could not attach thread for upcall. JNI error 
code: %d", result);

I'm assuming you don't have a mechanism for conveying an error back to the 
original entry point used by the native thread? Attaching an existing thread 
should only fail if we run out of C-Heap, so we're on the brink of aborting 
anyway, but still the guarantee here is not ideal.

Yeah, we have no idea where/how to report such and error. The native code might 
just call a function through a function pointer like `void(*)(void)` i.e. no 
place to return an error code. Also, since it's a previously non-attached 
thread, there is no JavaFrameAnchor that gives us a last Java frame we could 
jump back to and throw an exception.

Yeah not a solvable problem when you can transparently attach to the VM.

Is there's a more explicit way to exit with an error, other than a `guarantee` 
that you would prefer here?

Perhaps vm_exit_out_if_memory under the assumption that is the only reason attach could fail. But really I don't think it makes much practical difference.

Thanks,
David
-----

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

PR: https://git.openjdk.java.net/jdk/pull/634

Reply via email to