[foreign-memaccess+abi] RFR: 8267989: Exceptions thrown during upcalls should be handled [v2]

[Maurizio Cimadamore]

On Mon, 31 May 2021 18:43:17 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Hi,
>> 
>> This patch regularizes exception handling for exceptions thrown during upcalls. Exceptions thrown during upcalls are now always handled by printing out the stack trace and then calling `System::exit` (see the JBS issue for some motivation).
>> 
>> I've added some documentation for the exception handling to `CLinker::upcallStub`, as well as a new public `int` constant in `CLinker` which is the error code that is passed to `System::exit`. The returned error code can also be configured by a system property, which for now is mostly useful for testing purposes to make sure we don't get a consistent false positive.
>> 
>> Thanks,
>> Jorn
>
> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address review comments:
>   - tweak documentation
>   - remove system property
>   - simplify test & check stderr for stack trace message

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/CLinker.java line 119:

> 117:      * @see CLinker#upcallStub(MethodHandle, FunctionDescriptor, ResourceScope)
> 118:      */
> 119:     int ERR_UNCAUGHT_EXCEPTION = 1;

On a second thought, I'm not super sure of the value of documenting what the exit code should be? Maybe we can leave that unspecified?

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/ProgrammableUpcallHandler.java line 299:

> 297:             }
> 298:         } catch(Throwable t) {
> 299:             SharedUtils.handleUncaughtException(t);

Ah - missed that :-)

test/jdk/java/foreign/TestUpcallException.java line 49:

> 47: import static org.testng.Assert.fail;
> 48: 
> 49: public class TestUpcallException {

I wonder if we should also test with a security manager (even though that's getting deprecated) - and see what happens when you call System.exit and SM is enabled with policy to block off calls to System.exit.

Using `Shutdown.exit` might be safer, as it does not SM checks.

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

PR: https://git.openjdk.java.net/panama-foreign/pull/543