I''m using multiple threads, and synchronizing them via CRITICAL_SECTION objects. Why doesn''t the following code for getting a "safe" copy of a shared variable work correctly? The CRITICAL_SECTION object is already initialized properly, but that code is not shown here for clarity. int AccessVariable() { int temp = 0; EnterCriticalSection(&cs); temp = my_global_variable; LeaveCriticalSection(&cs); return temp; } Between the calls to EnterCriticalSection and LeaveCriticalSection, temp has the proper value -- the same as my_global_variable. At the "return temp;" statment, however, it''s automatically initialized to NULL. Why? - null_pointer

The critical section protect my_global_variable, not temp. You shouldn''t see this behavior at all. Methinks something''s screwy with your threads--maybe you accidently made temp static and it''s being changed by another thread?

I guess one thing to note is that you're not really return temp, you are returning a temporary object created by the compiler.
the declaration
int AccessVariable();

will automatically make a copy when returning it.

In your example case this is a good thing because you cannot return a reference to a local variable.

temp is probably going 'null' because the variable temp is going out of scope at the end of the return.

if you're just giving read access, you could try removing the critical sections on the accessor.

although maybe what is better is to make sure that the class only allows modification of data inside the class and does not provide accessors ( even ones that make copies of the object )
because once you get that copy of the variable and do something with it, it may have already been changed by another thread and now the value that you have copied from it is invalid.

Not that I really know the best solution to this, I'm kind of wrestling with a similar problem...

Edited by - SteveC on 2/1/00 5:02:43 PM

If you are only reading a shared variable, is it even necessary to put it in a critical section? Not a shared array, but just a shared int or something.

I''d generally say no. But I guess it might depend on what you''re using.

If you return by value, the variable will be copied to a temporary and the temporary will be returned.

If you return by reference ( hopefully const reference ) then sort of get the address to where that variable is.

The catch is that in either case when you go to refer to the original, either to use it as the source for copying or when somewhere is is using the reference to it, another thead may get scheduled and modify the value. If this happens ''in between'' your using it then there could be problems.

But maybe this won''t be a big deal for primitive types because you''ll only miss the write instruction of an assignment. Which is just looking at the variable before it has changed.

So far as I know any assignment is not going to be 1 atomic instruction. That''s why there''s things like the win32 api function InterlockedExchange.

For primitive types you are probably OK just returning either by reference or by value because the write can occur with one instruction.

For classes all bets are off. The object''s internal state is probably inconsistent if changes are being made to it in another thread.

Maybe the thing to consider is that if you need to synchronize on this fine a level of granularity, maybe there is another problem. Sometimes you can''t make every component ''do the right thing'' because you do not know the context of how it is being used. Even if you could, there''s probably huge amounts of overhead.

Look at STL, its not thread-safe but it is still extremely useful.

anyway, not that I''m the multi-thread expert...

Thank you for all of the replies!

Actually, this is just an oversimplified example that I wanted to use to test my problem. I didn''t know if LeaveCriticalSection would actually change the value of local variables...it''s probably my other code that''s not working properly.

You see, I''m using a linked list class encapsulated in a class called Queue which simply lets the main thread give its worker threads tasks asynchronously. To protect the linked list -- since the list is using classes for its data -- I had to use critical section objects in the Queue class. This keeps thread from reading and writing the linked list at the same time. The great need for this comes (as SteveC alluded to) because the linked list sometimes needs many lines of code to add or remove things, as opposed to just one line of code. I don''t want one thread to remove part of the list and another to add, say, 5 more items, and then another thread empties the list...you get the idea. The code to add and remove items in the list is very different depending on if there are 0, 1, or 2 or more items already in the list. Somewhere along the line I''d be using null_pointer''s!

By the way, I put a Windows EVENT as a data member of the Queue class, which is triggered and released whenever data is there. That lets Windows block threads efficiently when there''s no work to be done. All the threads have to do is check the event in a while() loop through the Queue class''s WaitForTask() function. Oh, and the event is manual reset so that it''s set whenever there''s valid data in the Queue.

Kind of neat but a little difficult to get working...anyway thanks for your help!

- null_pointer

You should look into the Win32 function
WaitForSingleObject. This will block your
thread until that particular event is
signalled. Much more efficient than spinning
through a while loop and continuously checking
the event.

Actually, I do use WaitForSingleObject. I got the code mixed up with my thread class''s IsDone() function which checks its own event to see if it should end. That''s the loop...I apologize if I caused some confusion! And that''s a good point. hehe my first attempts at multithreading lead to programs that were VERY unresponsive.

- null_pointer