Project

Profile

Help

Bug #2327

closed

Deadlock for concurrent instantiation of Configuration and EnterpriseConfiguration

Added by Gunther Rademacher over 7 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Multithreading
Sprint/Milestone:
Start date:
2015-03-12
Due date:
% Done:

100%

Estimated time:
Legacy ID:
Applies to branch:
9.6
Fix Committed on Branch:
9.6
Fixed in Maintenance Release:
Platforms:

Description

Our test suite occasionally was hanging with some threads showing call stacks, where Saxon configuration objects were being instantiated. After some deeper analysis, I can now reproduce it with the attached test program.

In this setup one thread instantiates EnterpriseConfiguration (originally in order to proceed with activation of the license), while the other instantiates Configuration (originally on behalf or TransformerFactory.newInstance). The problem appears to be caused by

  • the former accessing EnterpriseConfiguration, then Configuration (as it is the super class), while

  • the latter accessing Configuration, then EnterpriseConfiguration (by Class.forName during static initialization),

and this turns out to produce a deadlock. Instantiation of ProfessionalConfiguration presumably would cause the same problem.

It can be worked around by accessing the Configuration Class object beforehand. We will be using the workaround now, but thought you might be interested.

The output of the attached test program is:

both threads alive after 2 sec
both threads alive after 4 sec
both threads alive after 6 sec
both threads alive after 8 sec
both threads alive after 10 sec
stack trace:
  InstantiationProblem$1.run(InstantiationProblem.java:18)
stack trace:
  java.lang.Class.forName0(Native Method)
  java.lang.Class.forName(Class.java:259)
  net.sf.saxon.Configuration.makeConfigurationClass(Configuration.java:4448)
  net.sf.saxon.Configuration.<clinit>(Configuration.java:113)
  InstantiationProblem$2.run(InstantiationProblem.java:24)

Best regards,

Gunther


Files

InstantiationProblem.java (1.39 KB) InstantiationProblem.java Reproduction of issue Gunther Rademacher, 2015-03-12 16:56
Actions #1

Updated by Michael Kay over 7 years ago

  • Subject changed from Dealock for concurrent instantiation of Configuration and EnterpriseConfiguration to Deadlock for concurrent instantiation of Configuration and EnterpriseConfiguration
  • Category set to Multithreading
  • Status changed from New to In Progress
  • Assignee set to Michael Kay
  • Priority changed from Low to Normal
Actions #2

Updated by Michael Kay over 7 years ago

Reproduced the problem on 9.6 (but not on 9.7). JUnit test case added: ThreadedKeyTest/testConcurrentInstantiation().

Actions #3

Updated by Michael Kay over 7 years ago

Which version of the JDK are you using? Oracle claim to have improved classloading in JDK 1.7 to reduce the possibilities for deadlock:

http://docs.oracle.com/javase/7/docs/technotes/guides/lang/cl-mt.html

Apart from that, it's not clear that there's much we can do about the problem - it seems outside our control.

Actions #4

Updated by Michael Kay over 7 years ago

  • Status changed from In Progress to Won't fix

I seem to be able to reproduce the problem running under JDK 1.6, but it doesn't show up under JDK 1.8. So I think I'm inclined to close it with no action, as a JVM issue.

Actions #5

Updated by Gunther Rademacher over 7 years ago

In my environment it is consistently reproducible with JDK 1.6, JDK 1.7, and JDK 1.8. The latest compiler and runtime that I tested was 1.8.0_31.

I tested on Windows 7 Enterprise, but the test suite usually runs on Redhat Linux and it originally happened there. We also added a specific test for the issue and found that on the build server it only reproduces in 70% of test runs. But this may be due to limitations of the build server with respect to multithreading, or depend on its load.

Actions #6

Updated by Michael Kay over 7 years ago

Thanks. Do you have any suggestions as to anything we might do about it?

The literature on deadlocks in class-loading is fairly mind-blowing. And my experience of this area is that if you make significant changes, you break something - it's an area where it's impossible for us to test the variety of class-loading strategies used in different environments such as Eclipse, Spring, JBoss, etc.

Actions #7

Updated by Gunther Rademacher over 7 years ago

I think I'd move the Class.forName outside of the static initialization by lazily initializing configurationClass. Have just tried this on 9.6.0.3 and it appears to solve the issue:

113c113
<     public static final Class<? extends Configuration> configurationClass = makeConfigurationClass();
---
>     private static final Class<? extends Configuration> configurationClass = null;
346a347,353
>     public static Class<? extends Configuration> getConfigurationClass() {
>       if (configurationClass == null) {
>          makeConfigurationClass();
>       }
>       return configurationClass;
>     }
>
348c355
<         //System.err.println("New configuration: " + configurationClass.getName());
---
>         //System.err.println("New configuration: " + getConfigurationClass().getName());
350c357
<             return configurationClass.newInstance();
---
>             return getConfigurationClass().newInstance();

Though I have only modified the Configuration class, did not check whether there are reasons for configurationClass to be public.

Actions #8

Updated by Michael Kay over 7 years ago

  • Status changed from Won't fix to In Progress

I'm thinking we should be able to do it differently. This dynamic loading of the class is a relic from when it was all driven by the edition.properties file. Now that we use preprocessing for logic that varies from one Saxon edition to another, we could simply do the initialization of configurationClass as a static constant, with different values per edition.

Actions #9

Updated by Gunther Rademacher over 7 years ago

That might indeed work, though it has the mutual reference at class loading time. I just ran my test with the initialization changed to

    public static final Class<? extends Configuration> configurationClass = com.saxonica.config.EnterpriseConfiguration.class;

and it did not produce a deadlock.

Actions #10

Updated by Michael Kay over 7 years ago

  • Status changed from In Progress to Resolved

OK. For 9.6, I'm changing makeConfigurationClass so that it has a static reference to the relevant Configuration subclass, controlled by preprocessor directives: the dynamic loading is a relic of an earlier design. Making the same change for makePlatform(). For 9.7, I'm going to put these edition-dependent intiializations into a separate module controlled by the build process.

Actions #11

Updated by Gunther Rademacher over 7 years ago

Thanks!

Actions #12

Updated by O'Neil Delpratt over 7 years ago

  • Status changed from Resolved to Closed
  • % Done changed from 0 to 100
  • Found in version changed from Saxon-EE 9.6.0.4J to 9.6
  • Fixed in version set to 9.6.0.5

Bug fix applied in the Saxon 9.6.0.5 maintenance release.

Actions #13

Updated by O'Neil Delpratt over 6 years ago

  • Sprint/Milestone set to 9.6.0.5
  • Applies to branch 9.6 added
  • Fix Committed on Branch 9.6 added
  • Fixed in Maintenance Release 9.6.0.5 added

Please register to edit this issue

Also available in: Atom PDF