[
https://issues.apache.org/jira/browse/SANDBOX-231?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12616530#action_12616530
]
Martin Kalén commented on SANDBOX-231:
--------------------------------------
Thank you for your feedback.
The synchronization target used for the currentTimeMillis field indeed looks
like a bug in the original code. Changing this to ThreadClockImpl.class in the
getUUIDTime does however not change the fact that Thread.start() can no longer
be performed on a thread where the run() method has already completed (same
IllegalStateExceptions, see the technote and recent JavaDoc for more
information).
I agree with Sebb that the workerSuspended needs to be made volatile and will
attach a new patch without this flaw.
However, I do not (yet?) see why the pattern from the technote using
wait+notify as a substitution for suspend+resume (or like the original code:
complete/end+re-start) is wrong. wait() and notify() both work on the Object's
monitor and are, as far as I understand, intended for cross-thread
communication on instances rather than explicit threads.
The way I read the technote Thread.interrupt() is recommended as a substitute
for Thread.stop on a thread that waits for long periods ("e.g., for input"). I
also read the original code as more of suspend/resume than explicit stop/start.
The start() call seemed to me as just a substitute for re-run/resume, therefore
I tried to follow the recommended patter for suspend/resume.
All feedback and/or corrections are welcome.
> Failing Tests
> -------------
>
> Key: SANDBOX-231
> URL: https://issues.apache.org/jira/browse/SANDBOX-231
> Project: Commons Sandbox
> Issue Type: Bug
> Components: Id
> Reporter: Donal Murtagh
> Priority: Critical
> Attachments: ThreadClockImpl_jdk6.patch
>
>
> I downloaded the HEAD of SVN, ran the tests and I get three failures
> (appended to end of message). Is there a more stable version that I can
> download/build?
> org.apache.commons.id.uuid.state.NodeTest
> testNodebyteArray(org.apache.commons.id.uuid.state.NodeTest)
> java.lang.IllegalThreadStateException
> at java.lang.Thread.start(Thread.java:571)
> at
> org.apache.commons.id.uuid.clock.ThreadClockImpl.getUUIDTime(ThreadClockImpl.java:174)
> at org.apache.commons.id.uuid.state.Node.getUUIDTime(Node.java:116)
> at
> org.apache.commons.id.uuid.state.NodeTest.testNodebyteArray(NodeTest.java:63)
> 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 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 junit.framework.TestSuite.runTest(TestSuite.java:208)
> at junit.framework.TestSuite.run(TestSuite.java:203)
> at
> org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130)
> at
> org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
> at
> org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:460)
> at
> org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:673)
> at
> org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:386)
> at
> org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:196)
> testGetUUIDTime(org.apache.commons.id.uuid.state.NodeTest)
> java.lang.IllegalThreadStateException
> at java.lang.Thread.start(Thread.java:571)
> at
> org.apache.commons.id.uuid.clock.ThreadClockImpl.getUUIDTime(ThreadClockImpl.java:174)
> at org.apache.commons.id.uuid.state.Node.getUUIDTime(Node.java:116)
> at
> org.apache.commons.id.uuid.state.NodeTest.testGetUUIDTime(NodeTest.java:104)
> 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 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 junit.framework.TestSuite.runTest(TestSuite.java:208)
> at junit.framework.TestSuite.run(TestSuite.java:203)
> at
> org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130)
> at
> org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
> at
> org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:460)
> at
> org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:673)
> at
> org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:386)
> at
> org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:196)
> testGetLastTimestamp(org.apache.commons.id.uuid.state.NodeTest)
> java.lang.IllegalThreadStateException
> at java.lang.Thread.start(Thread.java:571)
> at
> org.apache.commons.id.uuid.clock.ThreadClockImpl.getUUIDTime(ThreadClockImpl.java:174)
> at org.apache.commons.id.uuid.state.Node.getUUIDTime(Node.java:116)
> at
> org.apache.commons.id.uuid.state.NodeTest.testGetLastTimestamp(NodeTest.java:129)
> 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 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 junit.framework.TestSuite.runTest(TestSuite.java:208)
> at junit.framework.TestSuite.run(TestSuite.java:203)
> at
> org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130)
> at
> org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
> at
> org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:460)
> at
> org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:673)
> at
> org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:386)
> at
> org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:196)
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
