At first I was skeptical of this being an ABA problem as indicated by gabr. It seemed to me that: if one thread looks at the current Head
, and another thread pushes then pops; you're happy to still operate on the same Head
in the same way.
However, consider this from your Pop
method:
NewHead:=Result.Next;
if InterlockedCompareExchangePointer(Pointer(Head),NewHead,Result)=Result
- If the thread is swapped out after the first line.
- A value for
NewHead
is stored in a local variable. - Then another thread successfully pops the node this thread was targetting.
- It also manages to push the same node back, but with a different value for
Next
before the first thread resumes. - The second line will pass the comparand check allowing head to receive the
NewHead
value from the local variable. - However, the current value for
NewHead
is incorrect, thereby corrupting your stack.
There's a subtle variation on this problem not even covered by your test app. This problem isn't checked in your test app because you aren't destroying any nodes until the end of your test.
- Look at current head
- Another thread pops some nodes.
- The nodes are destroyed, and new nodes created and pushed.
- By the time your "looking thread" is active again, it could be looking at an entirely different node that is coincidentally at the same address.
- If you're popping, you might assign a garbage pointer to
Head
.
Apart form the above...
Looking at your test app there's also some really dodgy code. E.g.
You generate a "random number": J:=GetTickCount and 7;(*Get a 'random' number 0..7*)
.
- Do you realise how fast computers are?
- Do you realise that
GetTickCount
will generate reams of duplicates in a tight loop? - I.e. the numbers you generate will be nothing like random.
- And when comments don't agree with code, my spidey-sense kicks in.
You're allocating memory of a hard-coded size: GetMem(zTmp,12);(*Allocate new node*)
.
- Why aren't you using
SizeOf
? - You're using a multi-platform compiler.... The size of that structure can vary.
- There is absolutely zero reason to hard-code the size of the structure.
Right now, given these two examples, I wouldn't be entirely confident that there isn't also an error in your test code.