OpenJPA
  1. OpenJPA
  2. OPENJPA-1168

NPE in UUIDGenerator.initializeForType1()

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3.0, 2.0.0-M3
    • Component/s: lib
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      When UUIDGenerator.createType1() is called by more than one thread at nearly the same time AND UUIDGenerator.createType1() wasn't called previously, a small timing window exists where a NPE will result.

      Scenario:
      Thread 1 calls UUIDGenerator.createType1() and RANDOM == null so initializeForType1() is called. While that thread is in initializeForType1(), it sets the static variable RANDOM so that value is no longer null. Now Thread 2 calls UUIDGenerator.createType1() and RANDOM is no longer null, so it proceeds to the call System.arraycopy(IP, 0, uuid, 10, IP.length);. At this point Thread 1 hasn't got to initializing IP yet, so Thread 2 gets hits a NPE.

      1. OPENJPA-1168.patch
        3 kB
        Rick Curtis
      2. OPENJPA-1168.patch
        4 kB
        Rick Curtis

        Activity

        Rick Curtis created issue -
        Hide
        Rick Curtis added a comment -

        The patch includes a test case and the fix for this issue.

        Since this bug is a timing window, running this test case against an un-patched tree may work from time to time. Please let me know if you want me to post some more information in the test case about why I did what I did.

        • Rick
        Show
        Rick Curtis added a comment - The patch includes a test case and the fix for this issue. Since this bug is a timing window, running this test case against an un-patched tree may work from time to time. Please let me know if you want me to post some more information in the test case about why I did what I did. Rick
        Rick Curtis made changes -
        Field Original Value New Value
        Attachment OPENJPA-1168.patch [ 12412925 ]
        Rick Curtis made changes -
        Assignee Rick Curtis [ curtisr7 ]
        Patch Info [Patch Available]
        Hide
        Michael Dick added a comment -

        Hi Rick,

        I'm unable to hit the timing window you mention on my system, but your explanation is sound.

        That said there's still a window where IP can be initialized (non null) but not copied. Another thread could hit this window when IP was empty and get a non-unique id. It's probably safer to set a different flag at the end of the method and key off that instead of IP or RANDOM.

        Thanks for the patch though - I doubt this was easy to find.

        Show
        Michael Dick added a comment - Hi Rick, I'm unable to hit the timing window you mention on my system, but your explanation is sound. That said there's still a window where IP can be initialized (non null) but not copied. Another thread could hit this window when IP was empty and get a non-unique id. It's probably safer to set a different flag at the end of the method and key off that instead of IP or RANDOM. Thanks for the patch though - I doubt this was easy to find.
        Michael Dick made changes -
        Assignee Michael Dick [ mikedd ]
        Hide
        Rick Curtis added a comment -

        Nice catch Mike.

        I updated the patch as recommended.

        Show
        Rick Curtis added a comment - Nice catch Mike. I updated the patch as recommended.
        Rick Curtis made changes -
        Attachment OPENJPA-1168.patch [ 12413018 ]
        Rick Curtis made changes -
        Attachment OPENJPA-1168.patch [ 12412925 ]
        Hide
        Rick Curtis added a comment -

        Fixed a problem in the test case that was introduced because of the previous patch.

        Show
        Rick Curtis added a comment - Fixed a problem in the test case that was introduced because of the previous patch.
        Rick Curtis made changes -
        Attachment OPENJPA-1168.patch [ 12413032 ]
        Hide
        Milosz Tylenda added a comment -

        This reminds me of a test case which sometimes fails for me while running against MySQL and which I did not have time to look at. Maybe this is connected although the test case is single-threaded. The stack trace I am receiving is:

        testDefaultValues(org.apache.openjpa.persistence.generationtype.TestGeneratedValues) Time elapsed: 0.787 sec <<< FAILURE!
        junit.framework.AssertionFailedError
        at junit.framework.Assert.fail(Assert.java:47)
        at junit.framework.Assert.assertTrue(Assert.java:20)
        at junit.framework.Assert.assertFalse(Assert.java:34)
        at junit.framework.Assert.assertFalse(Assert.java:41)
        at org.apache.openjpa.persistence.generationtype.TestGeneratedValues.testDefaultValues(TestGeneratedValues.java:49)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:585)
        at junit.framework.TestCase.runTest(TestCase.java:154)
        at junit.framework.TestCase.runBare(TestCase.java:127)
        at org.apache.openjpa.persistence.test.PersistenceTestCase.runBare(PersistenceTestCase.java:466)
        at junit.framework.TestResult$1.protect(TestResult.java:106)
        at junit.framework.TestResult.runProtected(TestResult.java:124)
        at junit.framework.TestResult.run(TestResult.java:109)
        at junit.framework.TestCase.run(TestCase.java:118)
        at org.apache.openjpa.persistence.test.PersistenceTestCase.run(PersistenceTestCase.java:181)
        at junit.framework.TestSuite.runTest(TestSuite.java:208)
        at junit.framework.TestSuite.run(TestSuite.java:203)
        at sun.reflect.GeneratedMethodAccessor58.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:585)
        at org.apache.maven.surefire.junit.JUnitTestSet.execute(JUnitTestSet.java:213)
        at org.apache.maven.surefire.suite.AbstractDirectoryTestSuite.executeTestSet(AbstractDirectoryTestSuite.java:140)
        at org.apache.maven.surefire.suite.AbstractDirectoryTestSuite.execute(AbstractDirectoryTestSuite.java:127)
        at org.apache.maven.surefire.Surefire.run(Surefire.java:177)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:585)
        at org.apache.maven.surefire.booter.SurefireBooter.runSuitesInProcess(SurefireBooter.java:345)
        at org.apache.maven.surefire.booter.SurefireBooter.main(SurefireBooter.java:1009)

        The TestGeneratedValues.java:49 contains:

        assertFalse(gv.getUuidstring().equals(gv2.getUuidstring()));

        This means UUID generator sometimes generates duplicates.

        Show
        Milosz Tylenda added a comment - This reminds me of a test case which sometimes fails for me while running against MySQL and which I did not have time to look at. Maybe this is connected although the test case is single-threaded. The stack trace I am receiving is: testDefaultValues(org.apache.openjpa.persistence.generationtype.TestGeneratedValues) Time elapsed: 0.787 sec <<< FAILURE! junit.framework.AssertionFailedError at junit.framework.Assert.fail(Assert.java:47) at junit.framework.Assert.assertTrue(Assert.java:20) at junit.framework.Assert.assertFalse(Assert.java:34) at junit.framework.Assert.assertFalse(Assert.java:41) at org.apache.openjpa.persistence.generationtype.TestGeneratedValues.testDefaultValues(TestGeneratedValues.java:49) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:585) at junit.framework.TestCase.runTest(TestCase.java:154) at junit.framework.TestCase.runBare(TestCase.java:127) at org.apache.openjpa.persistence.test.PersistenceTestCase.runBare(PersistenceTestCase.java:466) at junit.framework.TestResult$1.protect(TestResult.java:106) at junit.framework.TestResult.runProtected(TestResult.java:124) at junit.framework.TestResult.run(TestResult.java:109) at junit.framework.TestCase.run(TestCase.java:118) at org.apache.openjpa.persistence.test.PersistenceTestCase.run(PersistenceTestCase.java:181) at junit.framework.TestSuite.runTest(TestSuite.java:208) at junit.framework.TestSuite.run(TestSuite.java:203) at sun.reflect.GeneratedMethodAccessor58.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:585) at org.apache.maven.surefire.junit.JUnitTestSet.execute(JUnitTestSet.java:213) at org.apache.maven.surefire.suite.AbstractDirectoryTestSuite.executeTestSet(AbstractDirectoryTestSuite.java:140) at org.apache.maven.surefire.suite.AbstractDirectoryTestSuite.execute(AbstractDirectoryTestSuite.java:127) at org.apache.maven.surefire.Surefire.run(Surefire.java:177) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:585) at org.apache.maven.surefire.booter.SurefireBooter.runSuitesInProcess(SurefireBooter.java:345) at org.apache.maven.surefire.booter.SurefireBooter.main(SurefireBooter.java:1009) The TestGeneratedValues.java:49 contains: assertFalse(gv.getUuidstring().equals(gv2.getUuidstring())); This means UUID generator sometimes generates duplicates.
        Michael Dick made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Donald Woods made changes -
        Fix Version/s 2.0.0-M3 [ 12314148 ]
        Fix Version/s 2.0.0 [ 12314019 ]
        Donald Woods made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Michael Dick
            Reporter:
            Rick Curtis
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development