The explanation of this question says:
This seems absolutely spot on. So there are two issues, the uniqueness of the results and the ordering of the results. Next part:The order of the numbers or the actual numbers that are printed cannot be determined. There are 10 threads that will be executing the run() method and any thread may run at any time. Therefore, it is possible that a thread may increment threadcounter (say, from 1 to 2) and before it gets a chance to print the value, it is swapped out and another thread starts executing the method (thereby, incrementing threadcounter from 2 to 3, and printing 3). Now, if the earlier thread executes, it will print 3. Thus, 3 will be printed twice and 2 will never be printed.
This suggests that adding the keyword volatile to the threadcounter, that would fix the problem of uniqueness.Further, since the threadcounter is not declared as volatile, the updates made by one thread may not be visible to another thread. It is, therefore, possible that each of the ten threads see its value as 0 and update it to 1. Thus, one possible outcome could be that 1 is printed 10 times!
I added a little sleep() within run() method to test this, I hope this is a representative test. Without this, I always got unique and perfectly ordered results just because of chance, I guess.
Code: Select all
try
{
this.sleep(1000);
} catch (InterruptedException e) {
e.printStackTrace();
}
I still got the same numbers more than once after making the int volatile and also the ordering wasn't incremental. The suggested solution proved not to be a real solution to any of both issues, assuming my sleep() addition didn't trouble the results.
I guess that's because volatile prevents order-of-operation optimization within the processor or at JRE level but it doesn't merge "retrieving", "updating" and then "retrieving again" to one single operation. They're still separate things. I might very well be wrong in this but right now, it makes sense to me.
Next part:
So additionally to my this.sleep() method and the volatile keyword, I made the run method synchronized by adding that keyword to the method.To fix the problem, you can make the run method synchronized so that only one thread will try to update threadcounter at a time. Another thread will get a chance only after one thread exits out of the run method.
It doesn't seem to help much in either the fact that results are still not unique after this and also they still come in an undeterministic order.
I think it has to do with the fact that the monitor is then on the instance of my class while the threadcounter is a static variable.
If I remove the synchronized keyword from the run() method, and instead, add synchronized(TestClass.class), that does solve both the problems of uniqueness of the results and ordering of the results. I also actually have to wait a second for each result to show up with this modification. It makes sense to me, the monitor is now static so whichever instance there is, has to consider that monitor.
The final part of the explanation:
The AtomicInteger solution by itself seems to only fix the issue of uniqueness, not the ordering of the results. This makes sense to me.You may also use AtomicInteger instead of int and use its getAndIncrement method instead of the post increment operator.
I think I learned a lot by this question, the explanation and testing things out. I'd be happy to hear if I made any wrong assumptions or if there's anything more to take from this.