Uploaded image for project: 'jBPM'
  1. jBPM
  2. JBPM-3456

getDefaultMappings() static method is not thread-safe

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Obsolete
    • Icon: Critical Critical
    • jBPM 3.2.10
    • jBPM 3.2.2
    • None
    • None
    • Low
    • Hide

      start 2 threads calling the parseInputStream() concurrently before the mappings have been registered to the class.

      Show
      start 2 threads calling the parseInputStream() concurrently before the mappings have been registered to the class.

      We get the following stack when multiple threads calls jBPM after the JVM is started.

      Caused by: org.jbpm.JbpmException: no ObjectInfo class specified for element 'jbpm-context' 
            at org.jbpm.configuration.ObjectFactoryParser.parse(ObjectFactoryParser.java:139) 
            at org.jbpm.configuration.ObjectFactoryParser.parseElements(ObjectFactoryParser.java:117) 
            at org.jbpm.configuration.ObjectFactoryParser.parseElementsFromResource(ObjectFactoryParser.java:105) 
            at org.jbpm.JbpmConfiguration.parseObjectFactory(JbpmConfiguration.java:313) 
            at org.jbpm.JbpmConfiguration.parseInputStream(JbpmConfiguration.java:367
      

      When we breakpoint in the code, we figured out the defaultMappings was half initialized. The root cause is the method getDefaultMappings which is currently not thread-safe. Setting the static defaultMappings field should not occur until the map is mappings have been all added. The method is by design (performance optimized to NOT be synchronized), but to keep it non-blocking, the map in the operation should be moved at the end of the mappings initialization, so another thread will not retrieve a map half-way initialized.

      CURRENT:

      public static Map getDefaultMappings() {
          if (defaultMappings==null) {
            defaultMappings = new HashMap(); // from now on, other thread calling getDefaultMappings will "think" map is initialized.
            addMapping(defaultMappings, "bean",         BeanInfo.class);
            addMapping(defaultMappings, "ref",          RefInfo.class);
            addMapping(defaultMappings, "list",         ListInfo.class);
            addMapping(defaultMappings, "map",          MapInfo.class);
            addMapping(defaultMappings, "string",       StringInfo.class);
            addMapping(defaultMappings, "int",          IntegerInfo.class);
            addMapping(defaultMappings, "integer",      IntegerInfo.class);
            addMapping(defaultMappings, "long",         LongInfo.class);
            addMapping(defaultMappings, "float",        FloatInfo.class);
            addMapping(defaultMappings, "double",       DoubleInfo.class);
            addMapping(defaultMappings, "char",         CharacterInfo.class);
            addMapping(defaultMappings, "character",    CharacterInfo.class);
            addMapping(defaultMappings, "boolean",      BooleanInfo.class);
            addMapping(defaultMappings, "true",         BooleanInfo.class);
            addMapping(defaultMappings, "false",        BooleanInfo.class);
            addMapping(defaultMappings, "null",         NullInfo.class);
            addMapping(defaultMappings, "jbpm-context", JbpmContextInfo.class);
            addMapping(defaultMappings, "jbpm-type",    JbpmTypeObjectInfo.class);
          }
          return defaultMappings;
        }
      

      EXPECTED:

      public static Map getDefaultMappings() {
          if (defaultMappings==null) {
            HashMap initMappings = new HashMap(); // use local map so another thread calling getDefaultMappings will at worse instantiate another map but not use a half-initialized map
            addMapping(initMappings, "bean",         BeanInfo.class);
            addMapping(initMappings, "ref",          RefInfo.class);
            addMapping(initMappings, "list",         ListInfo.class);
            addMapping(initMappings, "map",          MapInfo.class);
            addMapping(initMappings, "string",       StringInfo.class);
            addMapping(initMappings, "int",          IntegerInfo.class);
            addMapping(initMappings, "integer",      IntegerInfo.class);
            addMapping(initMappings, "long",         LongInfo.class);
            addMapping(initMappings, "float",        FloatInfo.class);
            addMapping(initMappings, "double",       DoubleInfo.class);
            addMapping(initMappings, "char",         CharacterInfo.class);
            addMapping(initMappings, "character",    CharacterInfo.class);
            addMapping(initMappings, "boolean",      BooleanInfo.class);
            addMapping(initMappings, "true",         BooleanInfo.class);
            addMapping(initMappings, "false",        BooleanInfo.class);
            addMapping(initMappings, "null",         NullInfo.class);
            addMapping(initMappings, "jbpm-context", JbpmContextInfo.class);
            addMapping(initMappings, "jbpm-type",    JbpmTypeObjectInfo.class);
            defaultMappings = initMappings; // from now on, if another thread access the defaultMapping it won't be null and properly initialized
          }
          return defaultMappings;
        }
      

      We cannot migrate to 4x jBPM version, since it is not backward compatible with our 3.2.2 BPM processes. Can you please fix this in the 3.2.x branch?

              marco.rietveld Marco Rietveld (Inactive)
              marsteau_jira Philippe Marsteau (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

                Created:
                Updated:
                Resolved:

                  Estimated:
                  Original Estimate - 1 day
                  1d
                  Remaining:
                  Remaining Estimate - 1 day
                  1d
                  Logged:
                  Time Spent - Not Specified
                  Not Specified