Hi, So I've been following along with the series and Casey introduced the concept of a work queue and the fact that a thread could try to pull info off of it before it was actually filled out.

The way he proposed to solve that was using a write barrier like so

internal void
PushString(char *String)
{
    Assert(EntryCount < ArrayCount(Entries));

    // TODO(casey): These writes are not in order!
    work_queue_entry *Entry = Entries + EntryCount;
    Entry->StringToPrint = String;
	
    CompletePastWritesBeforeFutureWrites
	
    EntryCount++;
}

Even after this modification I still have the issue occur like so

Thread 4: String 0

Thread 3:

Thread 1: String 3

Thread 2: String 2

Thread 4: String 6

Thread 3: String 8

Thread 4: String 9

Thread 1: String 7

Thread 6: String 1

Thread 0:

Why is this still happening? Shouldn't the write barrier make this impossible? I am assuming this is exactly the issue where the thread prints out the string before it is actually filled out, but maybe something else is going on? To be clear I tried this at first with my own code and also with Casey's exact code from those days and I still have the issue. Why do I have this issue with the exact same code when he didn't?

After looking at it for a while I was wondering if the problem was coming from this part of the code

DWORD WINAPI
ThreadProc(LPVOID lpParameter)
{
    win32_thread_info *ThreadInfo = (win32_thread_info *)lpParameter;

    for(;;)
    {
        if(NextEntryToDo < EntryCount)
        {
            // TODO(casey): This line is not interlocked, so two threads could see the same value.
            // TODO(casey): Compiler doesn't know that multiple threads could write this value!
	    int EntryIndex = InterlockedIncrement((LONG volatile*)&NextEntryToDo) - 1;
	    CompletePastReadsBeforeFutureReads
            //int EntryIndex = NextEntryToDo++;

            // TODO(casey): These reads are not in order!
            work_queue_entry *Entry = Entries + EntryIndex;

            char Buffer[256];
            wsprintf(Buffer, "Thread %u: %s\n", ThreadInfo->LogicalThreadIndex, Entry->StringToPrint);
            OutputDebugStringA(Buffer);
        }
    }

//    return(0);
}

Can two threads see NextEntryToDo < EnrtyCount and enter here? I know we have an InterlockedIncrement on NextEntryToDo, but does that prevent more than one thread from entering the if block?

Also just to note, I couldn't format the code proper so I am sorry if it is hard to read I don't know how to make it proper on here.

To format the code you need to use markdown syntax. For code it's using 3 back quotes ( ` ) followed by the name of the language (cpp or c in this case) without space to start the block and 3 back quotes to end the code.

I was going to say that MSVC _WriteBarrier is deprecated and may not do anything on recent compiler versions, but I ran the code without the CompletePastWritesBeforeFutureWrites and the everything was still working properly. I used code from day 123 with modifications to reflect the code you provided (Windows 10, intel i5 2500).

Do you have the issue every time or is it working most of the time ? Did you download the code from Casey or did you type the same code ?

Could you provide the full "win32_handmade.cpp" that has the issue so that I can try to run it ?

I'm not very familiar with multi threading. I suppose the issue with the "if" you mentioned is possible but I believe you would still get every string printed once and some thread would print data from uninitialized entries (because of the interlocked increment) ?

Also it'll probably be addressed in later episode, when Casey starts using InterlockedCompareExchange.

Thank you for your help with the formatting.

So It actually doesn't happen every time. Sometimes all the strings are filled out and sometimes some random amount of them are missing. I should also note that I modified the thread count to be 8 even though in the source it was 4 i think because more threads makes the issue occur more frequently.

I was also thinking it probably gets addressed later, but I just decided to still ask just in case.

Here is the file just in case I somehow still mistyped something. I know it's a little awkward because it's the day 123 code just with the memory fences added in.

win32_handmade.cpp

Initially I was using my own typed code but after I tried the code from the source archive and it still was behaving wrong. It is possible I messed up in both though.

I was able to reproduce the issue twice, but it displayed every string once, and had a single empty line at the end, so it's not exactly what you got but seems to be the "if" issues you mentioned.

I had to run it a lot to reproduce so Casey probably had the issue, but it didn't show up.

Well as it turns out Day 126 addresses this issue, it is exactly that if(NextEntryToDo < EntryCount)

that is the issue and InterlockedCompareExchange solves it.

Also it seems to be displaying every string only once too that's working all good. I jumped the gun on this post for sure but thank you for your help mrmixer.