Hi,

I have some comments inline

On 2021/04/08 03:28:10, Jaroslav Tulach <[email protected]> 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: [email protected]
> > For additional commands, e-mail: [email protected]
> > 
> > For further information about the NetBeans mailing lists, visit:
> > https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
> 
> 
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
> 
> For further information about the NetBeans mailing lists, visit:
> https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
> 
> 
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists



Reply via email to