Hi Matthias, Thanks, I missed this message, but the good thing is that by looking into it already, I could understand your PR more easily :) This seems a trivial fix, but of course it needs more review and testing. I don't see major issues with this one, so I hope we can integrate that in a few days.
- Johan On 2021/04/08 11:19:34, Matthias Bläsing <mblaes...@doppel-helix.eu> wrote: > Hi Johan, > > thanks for chiming in. I requested an issue yesterday evening, but is > was not yet made public. Maybe you could see if you can open up > 9069811. It is full version of the report and can be considered a > duplicate of JDK-8264886. It contains the crash info and instructions > to reproduce though. > > I created a PR with a reproducer and a fix here: > > https://github.com/openjdk/jfx/pull/458 > > The code to manually reproduce can be found here: > > https://github.com/matthiasblaesing/reproduce-openjfx-crash2/tree/287ec203da986dc761551687017d973d1b2fe87e > (this is current master) > > In the referenced issue a description is given what is needed to use > the reproducer. > > We can also take the discussion to the openjfx mailing list, as I'm > subscribed. > > Greetings > > Matthias > > > Am Donnerstag, den 08.04.2021, 07:56 +0000 schrieb Johan Vos: > > Hi, > > > > I have some comments inline > > > > On 2021/04/08 03:28:10, Jaroslav Tulach <jaroslav.tul...@gmail.com> wrote: > > > Thanks a lot guys for investigating JavaFX integration into NetBeans! I > > > still > > > love to use HTML/Java whenever possible and having a working WebView > > > integration is essential for that... > > > > > > > Some comments below > > > > > > > > On 07/04/2021 8:21, Matthias Bläsing wrote: > > > > > Hi Antonio, > > > > > > > > > > Am Dienstag, den 06.04.2021, 23:24 +0200 schrieb antonio: > > > > > [...] > > > > > Here is the stack trace for it (from hs_err_pid*): > > > > > > > > > > Stack: [0x00007fa41ce47000,0x00007fa41d646000], > > > > > sp=0x00007fa41d6449c0, > > > > > free space=8182k Native frames: (J=compiled Java code, A=aot compiled > > > > > Java code, j=interpreted, Vv=VM code, C=native code) V > > > > > [libjvm.so+0x91203a] jni_CallStaticBooleanMethodV+0x7a > > > > > C [libjfxwebkit.so+0x5fd155] > > > > > JNIEnv_::CallStaticBooleanMethod(_jclass*, > > > > > _jmethodID*, ...)+0x85 C [libjfxwebkit.so+0x2a0d82a] > > > > > WTF::FileSystemImpl::makeAllDirectories(WTF::String const&)+0xda > > > > It seems "makeAllDirectories" > > > > (https://github.com/openjdk/jfx/blob/e0ce73a3c8d82d3274bd10799b530f397a90ba6 > > > > 0/modules/javafx.web/src/main/native/Source/WTF/wtf/java/FileSystemJava.cpp# > > > > L143) being invoked by a native thread does not use > > > > jvm->AttachCurrentThread. We'll have unexpected behaviour. > > > > > > > > This is a bug in libjfxwebkit that you may want to report. > > > > > > Right, as far as I know, the AttachCurrentThread call is prerequisite for > > > making calls to JVM. > > > > At first sight, this looks indeed something that needs to be fixed in > > OpenJFX. I filed an issue for that: > > https://bugs.openjdk.java.net/browse/JDK-8264886 > > > > > > > > > > C [libjfxwebkit.so+0x553707] > > > > > WebCore::StorageSyncManager::fullDatabaseFilename(WTF::String > > > > > const&)+0x27 C [libjfxwebkit.so+0x54e83a] > > > > > WebKit::StorageAreaSync::openDatabase(WebKit::StorageAreaSync::OpenDataba > > > > > seParamType)+0x3a C [libjfxwebkit.so+0x54f8a9] > > > > > WebKit::StorageAreaSync::performImport()+0x29 C > > > > > [libjfxwebkit.so+0x553f04] > > > > > WebCore::StorageThread::threadEntryPoint()+0xb4 C > > > > > [libjfxwebkit.so+0x29aa793] > > > > > WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*)+0x63 C > > > > > [libjfxwebkit.so+0x2a1253d] WTF::wtfThreadEntryPoint(void*)+0xd > > > > > > > > > > > > > > > There is no native frame and thus even if the thread itself would be > > > > > attached to the JVM as suggested in your second email, it would still > > > > > be missing a context to find that invoking java method and its class > > > > > loader. > > > > > > > > What people usually do when using JNI is to create global references to > > > > "jclass" and then reuse these global references in different threads. > > > > These are thread safe and avoid being finding classes and methods all > > > > the time. > > > > > > Yes, this is what native libraries should do. Before making calls from > > > Java to > > > JNI store references to essential Java classes (or object instances). > > > > That should be fixed indeed (as part of > > https://bugs.openjdk.java.net/browse/JDK-8264886 ) > > > > > > > a) On the JNI_OnLoad method (when the library is loaded) people do a > > > > "FindClass" as usual, and then create a new global reference to the > > > > jclass for future use in different threads, and store it in a global C > > > > variable. Could be something like: > > > > > > > > static jclass MyWhateverClass = NULL; > > > > > > > > ... > > > > > > > > JNI_OnLoad(...) { > > > > // Find the class in OnLoad > > > > jclass clazz = env->FindClass(...); > > > > // And keep a NewGlobalRef for future use... > > > > MyWhateverClass = env->NewGlobalRef(clazz); > > > > ... > > > > > > That's the recommended approach. However it shall be stated that this is > > > creating a (class loading) memory leak - it is not going to be possible > > > to > > > unload the JavaFX JAR file classes once this variable is initialized. > > > Just FYI, > > > I think we can live with such behavior. > > > > > > > b) In the native thread people call jvm->AttachCurrentThread (mandatory) > > > > and can then use the MyWhateverClass without invoking "FindClass" again. > > > > > > +1 > > > > > > > These "jclass" allocated with NewGlobalRef are thread safe, and can be > > > > used by different native threads (invoking AttachCurrentThread first) > > > > without problems. > > > > > > > > People usually cache also "jmethodID"s for future use, so they don't > > > > have to lookup the methods on each call. These "jmethodID" are thread > > > > safe by default, so there's no need to create a NewGlobalRef for them. > > > > > > > > For details on what JNI stuff is or is not thread safe [1] gives a nice > > > > summary. > > > > > > > > So to summarize, I'm afraid they'll need to modify the code like so: > > > > > > A bugfix release of JavaFX would be the best solution. Hopefully it can > > > be > > > release soon. > > > > > > > a) Use AttachCurrentThread in native method invocations (they may be > > > > doing it elsewere, but probably not in the stack trace you're sending). > > > > > > > > b) Cache the jclass(es?) they're using in native threads, by allocating > > > > it(them?) in JNI_OnLoad using NewGlobalRef. > > > > > > > > c) Use these cached jclass(es?) in the methods invoked by native calls. > > > > > > > > Note that even if the bug reveals itself in NetBeans, it may happen > > > > anywhere else. > > > > > > Right. Enough to load JavaFX with own classloader (possible to do in unit > > > tests of JavaFX) and the error should reproduce. > > > > It would be great if someone can add a simple reproducible snippet at the > > JBS bug. > > > > > -jt > > > > > > > [1] > > > > https://latkin.org/blog/2016/02/01/jni-object-lifetimes-quick-reference/ > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > > To unsubscribe, e-mail: dev-unsubscr...@netbeans.apache.org > > > > For additional commands, e-mail: dev-h...@netbeans.apache.org > > > > > > > > For further information about the NetBeans mailing lists, visit: > > > > https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: dev-unsubscr...@netbeans.apache.org > > > For additional commands, e-mail: dev-h...@netbeans.apache.org > > > > > > For further information about the NetBeans mailing lists, visit: > > > https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@netbeans.apache.org > > For additional commands, e-mail: dev-h...@netbeans.apache.org > > > > For further information about the NetBeans mailing lists, visit: > > https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists > > > > > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@netbeans.apache.org > For additional commands, e-mail: dev-h...@netbeans.apache.org > > For further information about the NetBeans mailing lists, visit: > https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists > > > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@netbeans.apache.org For additional commands, e-mail: dev-h...@netbeans.apache.org For further information about the NetBeans mailing lists, visit: https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
