On Thu, Oct 15, 2020 at 12:12 PM <amiloslavs...@apache.org> wrote:
>
> Author: amiloslavskiy
> Date: Thu Oct 15 10:12:52 2020
> New Revision: 1882518
>
> URL: http://svn.apache.org/viewvc?rev=1882518&view=rev
> Log:
> JavaHL: Introduce tests showing JVM crashes with TunnelAgent
>
> The crashes will be addressed in subsequent commits.
>
> [in subversion/bindings/javahl]
> * tests/org/apache/subversion/javahl/BasicTests.java
> Add helper class 'TestTunnelAgent' to emulate server replies in test
> environment.
> Add new tests which currently causes JVM to crash:
> * testCrash_RemoteSession_nativeDispose
> * testCrash_RequestChannel_nativeRead_AfterException
> * testCrash_RequestChannel_nativeRead_AfterSvnError
A couple more small remarks here below ...
<snip>
> + private String readClient(ByteBuffer readBuffer)
> + throws IOException
> + {
> + readBuffer.reset();
> + request.read(readBuffer);
> +
> + final int offset = readBuffer.arrayOffset();
> + return new String(readBuffer.array(),
> + offset,
> + readBuffer.position() - offset);
> + }
> +
> + private void emulateServer(String serverMessage)
> + throws IOException
> + {
> + final byte[] responseBytes = serverMessage.getBytes();
> + response.write(ByteBuffer.wrap(responseBytes));
> + }
^^ some lines indented with tabs instead of spaces
<snip>
> + public void testCrash_RemoteSession_nativeDispose() {
^^ curly brace should be on next line
<snip>
> + // 'OperationContext::openTunnel()' doesn't 'NewGlobalRef()'
> callback returned by 'TunnelAgent.openTunnel()'.
> + // When GC runs, it disposes the callback. When JavaHL tries to call
> it in 'remote.dispose()', JVM crashes.
^^ The comment above, describing how the JVM crashes, refers to the
situation before you actually fixed that :-). Can you rephrase it a
bit, so it is still applicable even with the crash now fixed?
<snip>
> + public void testCrash_RequestChannel_nativeRead_AfterSvnError()
> + {
> + final String wcRoot = new File("tempSvnRepo").getAbsolutePath();
> +
> + final ScriptItem[] script = new ScriptItem[]{
> + // openRemoteSession
> + new ScriptItem(Actions.EMUL_SERVER, "( success ( 2 2 ( ) (
> edit-pipeline svndiff1 absent-entries commit-revprops depth log-revprops
> atomic-revprops partial-replay inherited-props ephemeral-txnprops
> file-revs-reverse ) ) ) "),
> + new ScriptItem(Actions.READ_CLIENT, "edit-pipeline"),
> + new ScriptItem(Actions.EMUL_SERVER, "( success ( ( ANONYMOUS )
> 36:0113e071-0208-4a7b-9f20-3038f9caf0f0 ) ) "),
> + new ScriptItem(Actions.READ_CLIENT, "ANONYMOUS"),
> + new ScriptItem(Actions.EMUL_SERVER, "( success ( ) ) ( success (
> 36:00000000-0000-0000-0000-000000000000 25:svn+test://localhost/test (
> mergeinfo ) ) ) "),
> + // checkout
> + new ScriptItem(Actions.READ_CLIENT, "( get-latest-rev ( ) ) "),
> + // Error causes a SubversionException to be created, which then
> + // skips closing the Tunnel properly due to 'ExceptionOccurred()'
> + // in 'OperationContext::closeTunnel()'.
> + // If TunnelAgent is unaware and calls
> 'RequestChannel.nativeRead()',
> + // it will either crash or try to use a random file.
> + new ScriptItem(Actions.EMUL_SERVER, "( success ( ( ) 0: ) ) (
> failure ( ( 160006 20:This is a test error 0: 0 ) ) ) "),
> + // TunnelAgent is not aware about the error and just keeps
> reading.
^^ same here, the comments describe the behavior before the fix, maybe
rephrase it so it's still valid after you fixed it.
--
Johan
