[PATCH] Fix JavaHL crash in TunnelAgent.CloseTunnelCallback after GC

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Fix JavaHL crash in TunnelAgent.CloseTunnelCallback after GC

Alexandr Miloslavskiy
Please find test snippet and patch attached.

[[[
Fix JavaHL crash in TunnelAgent.CloseTunnelCallback after GC

When jobject reference is kept across different JNI calls, a new global
reference must be requested with NewGlobalRef(). Otherwise, GC is free
to remove the object. Even if Java code keeps a reference to the object,
GC can still move the object around, invalidating the kept jobject,
which results in a native crash when trying to access it.

[in subversion/bindings/javahl]
* native/OperationContext.cpp
   (OperationContext::openTunnel): Add NewGlobalRef() for kept jobject.
   (OperationContext::closeTunnel): Add a matching DeleteGlobalRef().
]]]

patch.txt (1K) Download Attachment
JavaHL_Crash_RemoteSession_nativeDispose.java (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix JavaHL crash in TunnelAgent.CloseTunnelCallback after GC

James McCoy-3
On Fri, Aug 07, 2020 at 08:47:07PM +0200, Alexandr Miloslavskiy wrote:

> Please find test snippet and patch attached.
>
> [[[
> Fix JavaHL crash in TunnelAgent.CloseTunnelCallback after GC
>
> When jobject reference is kept across different JNI calls, a new global
> reference must be requested with NewGlobalRef(). Otherwise, GC is free
> to remove the object. Even if Java code keeps a reference to the object,
> GC can still move the object around, invalidating the kept jobject,
> which results in a native crash when trying to access it.
>
> [in subversion/bindings/javahl]
> * native/OperationContext.cpp
>   (OperationContext::openTunnel): Add NewGlobalRef() for kept jobject.
>   (OperationContext::closeTunnel): Add a matching DeleteGlobalRef().
> ]]]

Is this superceded by your other patch?

Cheers,
--
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix JavaHL crash in TunnelAgent.CloseTunnelCallback after GC

Alexandr Miloslavskiy
On 11.08.2020 3:56, James McCoy wrote:

> Is this superceded by your other patch?

No, these are two different patches.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix JavaHL crash in TunnelAgent.CloseTunnelCallback after GC

Nathan Hartman
In reply to this post by Alexandr Miloslavskiy
On Fri, Aug 7, 2020 at 3:07 PM Alexandr Miloslavskiy
<[hidden email]> wrote:

>
> Please find test snippet and patch attached.
>
> [[[
> Fix JavaHL crash in TunnelAgent.CloseTunnelCallback after GC
>
> When jobject reference is kept across different JNI calls, a new global
> reference must be requested with NewGlobalRef(). Otherwise, GC is free
> to remove the object. Even if Java code keeps a reference to the object,
> GC can still move the object around, invalidating the kept jobject,
> which results in a native crash when trying to access it.
>
> [in subversion/bindings/javahl]
> * native/OperationContext.cpp
>    (OperationContext::openTunnel): Add NewGlobalRef() for kept jobject.
>    (OperationContext::closeTunnel): Add a matching DeleteGlobalRef().
> ]]]

Ping... this is the 2nd patch. Review would be appreciated :-)

Thanks,
Nathan
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix JavaHL crash in TunnelAgent.CloseTunnelCallback after GC

Thomas Singer (SyntEvo)
In reply to this post by Alexandr Miloslavskiy
Has this patch been merged yet? If not, what input is needed to get it
accepted?

Tom


On 2020-08-07 20:47, Alexandr Miloslavskiy wrote:

> Please find test snippet and patch attached.
>
> [[[
> Fix JavaHL crash in TunnelAgent.CloseTunnelCallback after GC
>
> When jobject reference is kept across different JNI calls, a new global
> reference must be requested with NewGlobalRef(). Otherwise, GC is free
> to remove the object. Even if Java code keeps a reference to the object,
> GC can still move the object around, invalidating the kept jobject,
> which results in a native crash when trying to access it.
>
> [in subversion/bindings/javahl]
> * native/OperationContext.cpp
>    (OperationContext::openTunnel): Add NewGlobalRef() for kept jobject.
>    (OperationContext::closeTunnel): Add a matching DeleteGlobalRef().
> ]]]
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix JavaHL crash in TunnelAgent.CloseTunnelCallback after GC

Alexandr Miloslavskiy
In reply to this post by Alexandr Miloslavskiy
Now available on branch 'javahl-1.14-fixes', r1882522.