[PATCH] Fwd: JNI segfault while running Java tests

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

[PATCH] Fwd: JNI segfault while running Java tests

Daniel Sahlberg
Just a kind ping on this patch. I realise I mark the subject of the original message so maybe it went under the radar.

I also forgot the log message:
[[[
* subversion/bindings/javahl/native/jniwrapper/jni_string_map.hpp
  (Java::BaseImmutableMap::Entry::key): Create the String::Contents object with a projer object since it keeps a reference throughout the life of String::Contents. Fixes https://svn.haxx.se/dev/archive-2020-08/0010.shtml
]]]

The patch itself is the same as James verified.

Kind regards
Daniel

---------- Forwarded message ---------
Från: Daniel Sahlberg <[hidden email]>
Date: sön 9 aug. 2020 kl 23:10
Subject: Re: JNI segfault while running Java tests
To: Nathan Hartman <[hidden email]>
Cc: Subversion Development <[hidden email]>


Den sön 9 aug. 2020 kl 15:28 skrev Daniel Sahlberg <[hidden email]>:
Den sön 9 aug. 2020 15:14Nathan Hartman <[hidden email]> skrev:
On Sat, Aug 8, 2020 at 1:23 PM James McCoy <[hidden email]> wrote:
On Sat, Aug 08, 2020 at 10:35:14AM -0400, James McCoy wrote:
> The Debian builds for 1.14.0 recently started crashing while running the
> Java tests.  This is pretty far out of my expertise, so hopefully
> someone can help out.

I don't know if it's related, but a few days ago we received a patch from Alexandr Miloslavskiy to fix a crash which is caused by a garbage collected object not being pinned before use by native code [1]. Perhaps Alexandr found the issue because of a similar crash to the one you're experiencing. Could you try the patch?

I thought about the same. However the patch didn't seem to make a difference. 

I can confirm James' statement that it crashes when compiled using GCC 10 but it seems to work with GCC 9. In my case I'm using Fedora 32 versus Fedora 30 so I can't rule out that there are other differences but it seems reasonable that GCC is doing something strange. 

I have some done some preliminary investigations but I'm done yet, it seems that the code is using an object that has already been destructed. 

Kind regards 
Daniel 

I have investigated further and I think I have found the issue. A patch is attached, basically changing 
    const String::Contents key(String(m_env, jkey));
to 
    const String str(m_env, jkey);
    const String::Contents key(str);
in ImmutableMap.for_each.

If I understand things correctly (admittedly I'm not an expert in C++), the lifetime of the String object is just the execution of the constructor of the Contents class. But the Contents class saves a reference to the String object in a member variable. When the Contents object is destroyed at the end of the function, it references the already previously destroyed String object.

This is the same in GCC 9 as well as GCC 10 (also the same in Visual Studio 2019!) so I'm guessing that GCC 10 is better at "cleaing up" destroyed object to the point where it trigger a segfault (but it's not consistent as a "minimal example" with GCC 10 show this behaviour but still doesn't segfault).

When the String object is assigned to it's own variable it lives until the end of the function and it is destroyed after the Contents object, thus the destructor of the Contents class succeeds.

With this patch make check-javahl succeeds with GCC 10. I have also applied it in my GCC 9 build and all checks still succeed.

Kind regards
Daniel


jnistringmap_coredump.patch.txt (870 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fwd: JNI segfault while running Java tests

Nathan Hartman
On Fri, Aug 14, 2020 at 9:08 AM Daniel Sahlberg
<[hidden email]> wrote:
> Just a kind ping on this patch. I realise I mark the subject of the original message so maybe it went under the radar.

Committed in r1880886.

Thanks for your patch and good catch on this subtle error!

The patch looks good to me. Although I don't have the setup to observe
the crash, I studied the code and I agree with the analysis (yours and
Alexandr's) in the other thread [1].

Thanks again,
Nathan

[1] "JNI segfault while running Java tests" started by James McCoy on
2020/08/08 archived at:
https://lists.apache.org/thread.html/rff3fa5ea97267adf36cc9daa3be01392e53fbc8050a91df4bbac01d8%40%3Cdev.subversion.apache.org%3E
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fwd: JNI segfault while running Java tests

Daniel Shahaf-2
Nathan Hartman wrote on Sun, 16 Aug 2020 00:47 -0400:
> On Fri, Aug 14, 2020 at 9:08 AM Daniel Sahlberg
> <[hidden email]> wrote:
> > Just a kind ping on this patch. I realise I mark the subject of the original message so maybe it went under the radar.  
>
> Committed in r1880886.
>

Backport nomination?

Cheers,

Daniel


> Thanks for your patch and good catch on this subtle error!
>
> The patch looks good to me. Although I don't have the setup to observe
> the crash, I studied the code and I agree with the analysis (yours and
> Alexandr's) in the other thread [1].
>
> Thanks again,
> Nathan
>
> [1] "JNI segfault while running Java tests" started by James McCoy on
> 2020/08/08 archived at:
> https://lists.apache.org/thread.html/rff3fa5ea97267adf36cc9daa3be01392e53fbc8050a91df4bbac01d8%40%3Cdev.subversion.apache.org%3E

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fwd: JNI segfault while running Java tests

Nathan Hartman
On Sun, Aug 16, 2020 at 9:11 AM Daniel Shahaf <[hidden email]> wrote:

>
> Nathan Hartman wrote on Sun, 16 Aug 2020 00:47 -0400:
> > On Fri, Aug 14, 2020 at 9:08 AM Daniel Sahlberg
> > <[hidden email]> wrote:
> > > Just a kind ping on this patch. I realise I mark the subject of the original message so maybe it went under the radar.
> >
> > Committed in r1880886.
> >
>
> Backport nomination?

Nominated for backport to 1.14.x and 1.10.x.

Cheers,
Nathan