Win a copy of Learn Spring Security (video course) this week in the Spring forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Misbehaving Thread Code?

 
Jim Crawford
Ranch Hand
Posts: 127
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
It seems to me that the following thread code misbehaves.
See output at bottom of code:
package study;
/**
* <p>Title: </p>
* <p>Description: </p>
* <p>Copyright: Copyright (c) 2002</p>
* <p>Company: </p>
* @author JLM
* @version 1.0
*/

//====================================================================
public class LockInvestigation
{
//----------------------------------------------------------
public LockInvestigation()
{
AssignmentThread operator1 =
new AssignmentThread(5222222222222222225L);
AssignmentThread operator2 =
new AssignmentThread(5111111111111111115L);
operator1.start();
operator2.start();
System.out.println(" LockInvestigation Completed.");
}
}

//====================================================================
class AssignmentThread extends Thread
{
private long assignmentValue;
// This is a shared varible between threads
private static long testVariable;
// DEBUG
private volatile long testValue1;
//----------------------------------------------------------
public AssignmentThread(long value)
{
assignmentValue = value;
}
//----------------------------------------------------------
public void run()
{
for (int count=0; count<10000000; count++)
{
// Assign to the shared variable
testVariable = assignmentValue;
try
{
synchronized(Class.forName("study.ValueAssignmentThread"))
{
testValue1 = testVariable;
if (testVariable == 5222222222222222225L || testVariable == 5111111111111111115L)
// Even if there is a mistake, it would be a normal thread
// scheduling problem. We are interested in the long type
// assignment failing as a result of a special kind of failure
continue;
System.err.println(" Value that passed if (x): "+testValue1);
System.err.println(
" Result of second test (should be false):"+
"\n (testVariable == 5222222222222222225L || testVariable == 5111111111111111115L):"+
(testVariable == 5222222222222222225L || testVariable == 5111111111111111115L));
System.err.println(" Supposed to be same x: "+testVariable);
System.err.println();
}
}
catch (ClassNotFoundException e)
{
e.printStackTrace();
}
}
}
}
Expected output example (due to threads treating double and long non-atomically):

Value that passed if (x): 5222222224296276427
Result of second test (should be false):
(testVariable == 5222222222222222225L || testVariable == 5111111111111111115L):false
Supposed to be same x: 5222222224296276427
Bad output example:

Value that passed if (x): 5111111109037056913
Result of second test (should be false):
(testVariable == 5222222222222222225L || testVariable == 5111111111111111115L):true
Supposed to be same x: 5222222222222222225
Value that passed if (x): 5111111111111111115
Result of second test (should be false):
(testVariable == 5222222222222222225L || testVariable == 5111111111111111115L):true
Supposed to be same x: 5222222222222222225
 
Jim Crawford
Ranch Hand
Posts: 127
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I expected a reply by now, but maybe I'm just too anxious.
Please copy and paste the code and check it out if you think you know threads a bit (or more than a bit.. or whatever).
I'm a bit concerned over this. I think I did my locking propperly and still this weird result occurs. Please see if you know why.

I'm aware that the first assignment in the thread isn't thread safe, but that's the whole point. The thing is.... I don't think the code in the synchronized section ............................
... maybe this is exactly the problem. Sometimes it helps explaining your problem, then you sometimes see it yourself...
I thought that the value of 'testVariable' is guaranteed in the synchronized section. The thing is.. the other thread is free to assign to it at any time.... so... while the tests ('if's etc.) are going on in the synchronized section the value could, and is at times changed at the critical time.
This causes the 'if' fallthrough and later the surprise that the value isn't consistent with what the tested condition was.
mmm. :roll:
Interesting... Feel free to comment. Just don't call me a dumb a'' please. :-)
 
Barry Gaunt
Ranch Hand
Posts: 7729
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I would update your static variable in a synchronized static method and rely on the JVM do the locking of the class. All that Class.forName stuff makes me shudder...keep it simple and all that.
Sorry I'm supposed to be working, so I do not have time to analyze your code in detail.
-Barry
 
Jim Crawford
Ranch Hand
Posts: 127
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by Barry Gaunt:
I would update your static variable in a synchronized static method and rely on the JVM do the locking of the class. All that Class.forName stuff makes me shudder...keep it simple and all that.

I needed a class level lock... I reckon the JVM would have allocated the lock on class level....... not sure. I'm not quite clear on all the low level working, but I did study the Java Lang Spec a bit.
 
Barry Gaunt
Ranch Hand
Posts: 7729
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
So all you need is:

and perhaps a getter too:

[ September 09, 2002: Message edited by: Barry Gaunt ]
 
Ron Newman
Ranch Hand
Posts: 1056
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Class.forName("study.ValueAssignmentThread")

but I don't see a class with that name anywhere in your code.
If you know the class name at compile time, don't use Class.forName("Foo"). Use Foo.class instead.
 
Jose Botella
Ranch Hand
Posts: 2120
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello Jim.
Please ident your code withUBB Code.
This is a slighly modied version and output:

The first two paragraphs in the output correspond to a corrupted value assigned to a non volatile long field (testVariable) by two unsynchronized threads. Placing testVariable = assignmentValue; within the synchronized block solves this problem. It's possible also to declare testVariable volatile and no corrupted value appears in the output anymore.
This is how I think the last paragraph in the output was printed:
operator2 assigned 5111111111111111115 to testVariable. It enters the synchronized block and assigns testVariable to testValue1. But before testing the following if, operator1 tries to execute testVariable = assignmentValue;. The if statement and the later assignment are executed "at the same time" because there are not synchronized. operator1 assigns the first 32 bits and before assining the last 32, operator2 executes if with a value of testVariable that is neither 5111111111111111115 nor 5222222222222222225. Therefore operator2 doesn't execute continue but the rest of the pritn statements. However before operator2 executes System.out.println("testValue1 "+testValue1 + " testVariable " + testVariable); , operator1 has already finished its assignment and it's now blocked before the synchronized block. Now testVariable is 5222222222222222225, and the the output shows it.
[ September 10, 2002: Message edited by: Jose Botella ]
 
Fintan Conway
Ranch Hand
Posts: 142
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Question :
Does JDK 1.4 treat the volatile keyword correctly?
I know that some JVMs simply ignore volatile.
Thanks,
Fintan
 
Jim Crawford
Ranch Hand
Posts: 127
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Question :
Does JDK 1.4 treat the volatile keyword correctly?
I know that some JVMs simply ignore volatile.

That wouldn't be too nice of them. You can test it. Take the code I posted and make the static volitile. You wouldn't get the half long assigned values.
Like (unsynchronized):
x = 111111111111 (whatever.. make it max long size)
and x = 222222222222

then if the volitile didn't work you'd get some values like:
111111837773.
That means the assignment was non-atomic, which was the actual point of the program I wrote.
Cheers.
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic