On 29.10.2015 21:00, Remi Forax wrote:
Hi Jochen,
line 75:
atomicCategoryUsageCounter.incrementAndGet();
categoriesInUse = atomicCategoryUsageCounter.get();
you increment the counter and then get the value, but another thread can change
the counter in between,
in that case you end up with the wrong value.
yes, I know that. The question is what nature the wrong value has. The
method that checks if a category is in use in the current thread only
checks for categoriesInUse>0. So even if the number is wrong, as long as
it is >0 there is no problem. The exact number is stored in
atomicCategoryUsageCounter. I guess the code could be changed to use a
boolean instead and then it would be:
categoriesInUse = atomicCategoryUsageCounter.get() >0;
It would show the intent better I think... On the other hand, your
suggestion:
The fix is to ask for the value of the counter at the same time,
categoriesInUse = atomicCategoryUsageCounter.incrementAndGet();
Is more near the current logic and I really ask myself, why I forgot
that you can get the value here right away ;) That's a fix we can do
very easily, thanks.
line 99:
atomicCategoryUsageCounter.getAndDecrement();
categoriesInUse = atomicCategoryUsageCounter.get();
same issue, the fix is
categoriesInUse = atomicCategoryUsageCounter.decrementAndGet();
Note: I've replaced getAndDecrement by decrementAndGet to keep the same
semantics.
thanks, will do those changes, My gut feeling though is, that this is
not the real issue. But can't hurt to make the code more safe in this
manner, since it does not add new synchronization points.
bye blackdrag
