-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dead Lock in class initialization #652
Comments
We've had similar reports of a deadlock in the GStreamer bindings. Calling field.get(...) inside the synchronized block is really problematic in a large JNA codebase as it can set off a massive train of class initializations that can lead to deadlock with other threads. Is it possible to bring class initialization out of synchronized blocks at all? |
Perhaps invokeSynchronized needs to push its synchronization block down to a more primitive level, after its converted all of its arguments into most primitive form?On May 7, 2016, at 11:57 AM, Neil C Smith [email protected] wrote:We've had similar reports of a deadlock in the GStreamer bindings. Calling field.get(...) inside the synchronized block is really problematic in a large JNA codebase as it can set off a massive train of class initializations that can lead to deadlock with other threads. Is it possible to bring class initialization out of synchronized blocks at all?—You are receiving this because you are subscribed to this thread.Reply to this email directly or view it on GitHub |
In my opinion, that's not the issue. It would possibly solve @bschwert immediate problem, but the root cause is forcing class initialization with the libraries lock in Native. We don't actually use the ProxyObject stuff, incidentally, it's a deadlock with something else. |
I'm still trying to understand the need for the synchronized block, bug asuming this is "only" to protected the libraries map, I see this option: private static void loadLibraryInstance(Class<?> cls) {
if(cls == null) {
return;
}
synchronized(libraries) {
if(libraries.containsKey(cls)) {
return;
}
}
Object staticFieldValue = null;
try {
Field[] fields = cls.getFields();
for (int i = 0; i < fields.length; i++) {
Field field = fields[i];
if (field.getType() == cls
&& Modifier.isStatic(field.getModifiers())) {
// Ensure the field gets initialized by reading it
staticFieldValue = field.get(null);
break;
}
}
} catch (Exception e) {
throw new IllegalArgumentException("Could not access instance of "
+ cls + " (" + e + ")");
}
synchronized (libraries) {
if (!libraries.containsKey(cls)) {
libraries.put(cls, new WeakReference<Object>(staticFieldValue));
}
}
} At least this method should now be save - I did not offer this as a PR, as I'm new to the code and wonder whether this lock is really only held to protect the libraries map in this case or if there are side effects I'm overlooking. |
@matthiasblaesing Is this really a good idea? You separate the "contains" and the "put" calls in two different synchronized-blocks. So two threads can have a race condition now and both assume that the class isn't initialized and are both initialize the class and the last one wins with his entry in the libraries-map... or doesn't the duplicate initialization matter at all? |
a) in that code, first one wins, although it wouldn't matter ... So, if that is the only reason for the lock, maybe a ConcurrentHashMap |
Your are right. The VM initializes static members only once... :-] For your suggestion of a ConcurrentHashMap: I do not know the JNA code for long, but it seems that there are situations where 'libraries' and 'typeOptions' are written in conjunction and are protected together with the one monitor 'libraries'. I don't know wether that is a must be, but a ConcurrentHashMap instead of the monitor object would get rid of this consistent behaviour. |
I have done some experiments. And with the following class you can reproduce the deadlock of @bschwert quite reliable (tested with JDK1.7.0_79, 64 Bit, JDK1.8.0_66, 32 Bit and 64 Bit on Windows 7 Home Premium Service Pack 1):
So you get these thread dumps:
I have then implemented the proposal of @matthiasblaesing. And: the above stated deadlock is away... but: there seems to be a new one:
So Pointer is waiting for "Native" to be loaded. But "Native" class initialization does not end because of... what? initIDs() wants to access the class Pointer as well? Why is this a problem? (ok... the class Pointer isn't initialized until right now, but in the Java Sourcecode these bidirectional dependencies are not a problem, aren't they?) |
In general the synchronization is only there to protect the libraries map; Then, of course, it's important that the Native class is consistently On Mon, May 9, 2016 at 7:20 PM, Mathias Mehrmann [email protected]
|
I've gotten hit by this issue as well. After some research, I have a theory about what's happening, though I have no idea how to verify it. Section 5.5 of the JVM specification for Java 8 states:
In light of the above, consider the following sequence:
If I'm interpreting the JVM spec correctly, this results in a deadlock. Thread B is initializing Native and tries to initialize Pointer (because of LOAD_CREF for Pointer in initIDs()), but discovers that thread A is already initializing Pointer, so it waits for thread A to complete. Meanwhile, thread A is initializing Pointer and tries to initialize Native (because it needs to read the POINTER_SIZE field), but discovers that thread B is already initializing Native, so it waits for thread B to complete. This would also explain why there's no problem when there's only one thread in play, because point 3 from the JVM spec handles that case. Again, this is mostly guesswork, but it fits the available evidence. Anyone have an idea how we would find out for sure? |
Hi folks.
What I'm seeing is that I have two separate threads causing classloading of these two independently (see stacktraces below), where the first one ("pool-1-thread-3") has started initializing
I think that what's needed is to eliminate circular dependencies that exist at classloading time. |
I agree this should be fixed properly. Until that happens, I have been using a workaround and haven't seen any deadlocks since. The trick is to do something once, at a point before anything tries to use JNA, that causes these two classes to get initialized. In my case, I'm calling Native#getDefaultStringEncoding() in the main() of my application. This causes both classes to get initialized on the same thread before any other threads can compete to initialize them, thus avoiding the deadlock. |
One can do a similar workaround in Jenkins by passing -Dhudson.remoting.RemoteClassLoader.force=com.sun.jna.Native to the JRE when starting the Jenkins slave processes, which should result in much the same effect (I'm in the process of rolling out that change right now...). |
Isn't the |
It's possible... I arrived here because I found the similarity between what I was seeing and the @rdicroce's comment 5 Dec 2016 - that comment perfectly describes what I had. If that comment is inapplicable to the OP then my comment is similarly misplaced... That said, my inclination would be that resolving the circular dependencies such that there's a very clear hierarchy of classes & initialization would likely make it far more apparent where the runtime deadlock came from - circular dependencies make for much confusion. |
All of the native methods and initialization, locks and whatever were moved
into Native to make this sort of thing easier to manage.
Unfortunately, the native bits of Native need to have references to a
limited number of JNA classes (including Pointer) on the native side, and
of course, most JNA base classes need to access the native methods in
Native.
…On Tue, Apr 11, 2017 at 9:58 AM, Peter Darton ***@***.***> wrote:
It's possible...
I arrived here because I found the similarity between what I was seeing
and the @rdicroce <https://github.com/rdicroce>'s comment 5 Dec 2016
<#652 (comment)>
- that comment *perfectly* describes what I had. If that comment is
inapplicable to the OP then my comment is similarly misplaced...
That said, my inclination would be that resolving the circular
dependencies such that there's a very clear hierarchy of classes &
initialization would likely make it far more apparent where the runtime
deadlock came from - circular dependencies make for much confusion.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#652 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAuMraW6zRpC43_5-TgNqjoeehrkB2Mtks5ru4cHgaJpZM4IWOYq>
.
|
I'd like you to have a look at this branch: https://github.com/matthiasblaesing/jna/tree/unlockedLoad This branch is a proposal, to reduce the problem surface. There are two vectors addressed:
This causes an API break, removing the Pointer#SIZE member. Any thoughts on this? |
I haven't tested the changes but they look fine to me. Removing Pointer#SIZE is an API break but doesn't seem like a big deal to me since Native#POINTER_SIZE has existed since 4.0 according to Git. So anyone using Pointer#SIZE can just do search and replace and still be compatible with any 4.x version of JNA. |
What's status of that potential fix. Has anyone attempted to use it? |
After the release of 4.5.0 (just happened) I just merged the JNA-5.0.0 branch into master. As I did not hear any complaints, I'll asume this fixes the issue. If not, please reopen. I don't want to break API in each version, so this would be a good point in time to raise problems. |
Summary: context: java-native-access/jna#652 Reviewed By: jtorkkola fbshipit-source-id: 7e34248
is there a release planned with this? |
thanks, it seems to work |
java-native-access/jna#652 This issue was resolved and property SIZE does not exist with JNA version 5.6.0.
* Update JNA to resolve framework loading issue on Big Sur java-native-access/jna#1215 * Remove workaround initializing JNA early java-native-access/jna#652 This issue was resolved and property SIZE does not exist with JNA version 5.6.0. * Update NuProcess to 2.0.1 for compatibility with JNA * Build NuProcess with openjdk version "1.8.0_275" jetty/jetty.project#3244 * Cherry pick a2912b9 a2912b9
* Update JNA to resolve framework loading issue on Big Sur java-native-access/jna#1215 * Remove workaround initializing JNA early java-native-access/jna#652 This issue was resolved and property SIZE does not exist with JNA version 5.6.0. * Update NuProcess to 2.0.1 for compatibility with JNA * Build NuProcess with openjdk version "1.8.0_275" jetty/jetty.project#3244 * Cherry pick a2912b9 facebook@a2912b9
* Update JNA to resolve framework loading issue on Big Sur java-native-access/jna#1215 * Remove workaround initializing JNA early java-native-access/jna#652 This issue was resolved and property SIZE does not exist with JNA version 5.6.0. * Update NuProcess to 2.0.1 for compatibility with JNA * Build NuProcess with openjdk version "1.8.0_275" jetty/jetty.project#3244 * Cherry pick a2912b9 facebook@a2912b9
* Update JNA to resolve framework loading issue on Big Sur java-native-access/jna#1215 * Remove workaround initializing JNA early java-native-access/jna#652 This issue was resolved and property SIZE does not exist with JNA version 5.6.0. * Update NuProcess to 2.0.1 for compatibility with JNA * Build NuProcess with openjdk version "1.8.0_275" jetty/jetty.project#3244 * Cherry pick a2912b9 facebook@a2912b9
* Update JNA to resolve framework loading issue on Big Sur java-native-access/jna#1215 * Remove workaround initializing JNA early java-native-access/jna#652 This issue was resolved and property SIZE does not exist with JNA version 5.6.0. * Update NuProcess to 2.0.1 for compatibility with JNA * Build NuProcess with openjdk version "1.8.0_275" jetty/jetty.project#3244 * Cherry pick a2912b9 facebook@a2912b9
I have observed a dead lock in class initialization, if two threads try to initialize a COM-Connection. My analysis is based on version 4.1.0. I have observed some bigger delay (~6s) in Thread-1 before the lock, while the Application is found in the RunningObjectTable. This is because of doors, which delays until the login is shown, but register in the ROT before.
Thread-1 is in Native.loadLibraryInstance with lock on libraries but waits for DISPPARAMS containing class OleAuto:
sun.misc.Unsafe.ensureClassInitialized(Class) Unsafe.java (native)
sun.reflect.UnsafeFieldAccessorFactory.newFieldAccessor(Field, boolean) UnsafeFieldAccessorFactory.java:43
sun.reflect.ReflectionFactory.newFieldAccessor(Field, boolean) ReflectionFactory.java:140
java.lang.reflect.Field.acquireFieldAccessor(boolean) Field.java:1057
java.lang.reflect.Field.getFieldAccessor(Object) Field.java:1038
java.lang.reflect.Field.get(Object) Field.java:379
com.sun.jna.Native.loadLibraryInstance(Class) Native.java:447
com.sun.jna.Native.getLibraryOptions(Class) Native.java:508
com.sun.jna.Native.getStructureAlignment(Class) Native.java:596
com.sun.jna.Structure.setAlignType(int) Structure.java:251
com.sun.jna.Structure.(Pointer, int, TypeMapper) Structure.java:176
com.sun.jna.Structure.(Pointer, int) Structure.java:172
com.sun.jna.Structure.(int) Structure.java:159
com.sun.jna.Structure.() Structure.java:151
com.sun.jna.platform.win32.OleAuto$DISPPARAMS.() OleAuto.java:379
com.sun.jna.platform.win32.OleAuto$DISPPARAMS$ByReference.() OleAuto.java:359
com.sun.jna.platform.win32.COM.util.ProxyObject.oleMethod(int, Variant$VARIANT$ByReference, IDispatch, OaIdl$DISPID, Variant$VARIANT[]) ProxyObject.java:596
com.sun.jna.platform.win32.COM.util.ProxyObject.oleMethod(int, Variant$VARIANT$ByReference, IDispatch, String, Variant$VARIANT[]) ProxyObject.java:574
com.sun.jna.platform.win32.COM.util.ProxyObject.getProperty(Class, String, Object[]) ProxyObject.java:381
com.sun.jna.platform.win32.COM.util.ProxyObject.invokeSynchronised(Object, Method, Object[]) ProxyObject.java:260
com.sun.jna.platform.win32.COM.util.ProxyObject.invoke(Object, Method, Object[]) ProxyObject.java:220
[…]
Thread-2 tries to initialize Class OleAuto but this requires the libraries Lock:
com.sun.jna.Native.cacheOptions(Class, Map, Object) Native.java:1547
com.sun.jna.Native.loadLibrary(String, Class, Map) Native.java:428
com.sun.jna.platform.win32.OleAuto.() OleAuto.java:102
com.sun.jna.platform.win32.Variant$VARIANT.(String) Variant.java:198
com.sun.jna.platform.win32.COM.util.Convert.toVariant(Object) Convert.java:41
com.sun.jna.platform.win32.COM.util.ProxyObject.invokeMethod(Class, String, Object[]) ProxyObject.java:407
com.sun.jna.platform.win32.COM.util.ProxyObject.invokeSynchronised(Object, Method, Object[]) ProxyObject.java:267
com.sun.jna.platform.win32.COM.util.ProxyObject.invoke(Object, Method, Object[]) ProxyObject.java:220
The text was updated successfully, but these errors were encountered: