Solved

One thread locks and other races

Posted on 2000-03-17
30
252 Views
Last Modified: 2013-11-20
My application creates two threads. One to acquire data and one to display it. I used a synchronization method from "Multithreaded Programming With Win32" sync objects are initialized like this.
      InitializeCriticalSection(&g_CritSec1);
      g_NotEmptyEvent = CreateEvent(NULL,TRUE,FALSE,NULL);
      g_NotFullEvent = CreateEvent(NULL,TRUE,FALSE,NULL);
      BufferState = EMPTY;

in Data acquisition thread:
      do
      {
            while(1)
            {
                  EnterCriticalSection(&g_CritSec1);
                  if(BufferState == FULL)
                  //buffer has not been processed by display thread
                  {
                        // leave the criticalsection
                        LeaveCriticalSection(&g_CritSec1);
                        //and wait for g_NotFullEvent to be signaled
                        //by display thread                                                WaitForSingleObject(g_NotFullEvent,INFINITE);
                        //when signaled retest Bufferstate
                        continue;
                  }
                  //get out of loop and do acquisition
                  break;
            }
            //do some stuff to acquire the data here
            BufferState = FULL;            
            LeaveCriticalSection(&g_CritSec1);
            PulseEvent(g_NotEmptyEvent);

      }while(*pContinue);

in Data display thread:
      do{
            while(1)
            {
                  EnterCriticalSection(&g_CritSec1);
                  if(BufferState == EMPTY)
                  {
                        LeaveCriticalSection(&g_CritSec1);
                        WaitForSingleObject(g_NotEmptyEvent,INFINITE);
                        continue;
                  }
                  break;
            }
            //do some stuff to display data
            BufferState = EMPTY;
            LeaveCriticalSection(&g_CritSec1);
            PulseEvent(g_NotFullEvent);

      }while(*pContinue);

this runs for a while then the data acquisition thread stops and the display thread keeps running. It doesn't seem that this would be possible but that is what happens.
0
Comment
Question by:airek
  • 12
  • 7
  • 4
  • +2
30 Comments
 
LVL 11

Expert Comment

by:mikeblas
ID: 2630695
What does "the data acuqisition thread stops" mean?  It sounds like you don't have a good idea of what's happening. Is it blocking on something? Where?  Is it starved for time? Is it at a low priority?

If you break into the debugger while your thread is starved, where is the statement pointer?

..B ekiM
0
 
LVL 32

Expert Comment

by:jhance
ID: 2630940
About the only thing I can see here that would stop the acquisition thread is the EnterCriticalSection call.

Since you've only posted an excerpt of your code, my first suspicion is that you are calling EnterCriticalSection in your display thread and somehow missing a paired call to LeaveCriticalSection.  After that happens, the display thread continues to have the lock on the critical section and the acquisition thread is forever locked out.
0
 
LVL 3

Expert Comment

by:Try
ID: 2631210
I have found that many authors in giving examples of a certain function or API, will do so only to show workability of the function.  Many times the examples are not meant to be copied, nor used as they present them.  For this reason, I would caution you not to rely too much on what you may see or have read in a book.  Instead, try and get a good understanding on your own about what a certain function (etc.) does, and do some experimenting on your own before using them more seriously.

After years of programming and designing systems, simplicity still plays a major role for me in my work.  I say this because in looking at your code, a redesign of what you're trying to accomplish, could perhaps be achieved without the use of 'do/while' and embedded 'while' statements.

But let's look at the Critical Section (CS) aspect of what you're trying to accomplish.  After you have acquired a CS, you enter it to start accessing a shared resource that only your thread at the moment is allowed to do.  (Other threads waiting to enter, will block indefinitely until the CS becomes available, since there is no time-out parameter for relinguishing the CS.)  Once inside (and doing your work), you don't have to leave the CS (in a permanent way).  You can re-enter it over and over again while you're inside it by using:

BOOL TryEnterCriticalSection(LPCRITICAL_SECTION  lpCriticalSection);

Naturally, because an internal count is kept of how many times you have entered the CS, an equal amount of:

LeaveCriticalSection(LPCRITICAL_SECTION  lpCriticalSection);

must also be called, to bring the count down to zero.  At that point, knowing that you have finished with the CS, the last thing you do, is to call:

DeleteCriticalSection(LPCRITICAL_SECTION  lpCriticalSection);

to surrender ownership of it.


Work things over in your own mind of what you're trying to do, without trying to be too fancy about it.  Remember, simplicity will always get you there.  Afterwards, when things are working fine, feel free (if you have a need for doing so), to add a little optimization to your code.

The name of the game in this business, is, "To get the program working first; the next thing right after that, is to get it working correctly."

Re-think things out if you have to.  Don't be afraid, and don't be lazy!
0
 
LVL 32

Expert Comment

by:jhance
ID: 2631412
Try,

I'm trying to figure what new material you've added to this question over my comment other than your boasting about how long you been programming....



0
 
LVL 3

Expert Comment

by:Try
ID: 2632713
You have a problem "jhance"?

First of all, I wasn't talking to you; it was "airek" to whom I was directing my comment, and if you're worrying about whether I end up with the points, let me say it as EXPLICITLY as I can.

"I AM NOT INTERESTED IN POINTS!!!!!!!"

OK!!  I DON'T WANT ANY POINTS!!!!  Do you understand that now?

Take the points!  Take the credit!  Take everything you feel you might be cheated out of!!  Take it ALL!!!  I AM NOT INTERESTED IN ANY OF THOSE THINGS!!!

My one and ONLY interest is to help "airek" figure out for himself, where in his logic things might be off (since he didn't post much of his code for a fuller evaluation to be made).

Besides, I don't see where you had mentioned anything to him about 'TryEnterCriticalSection', which is integral and just as important when working with Critical Sections, as 'EnterCriticalSection'.  AAMOF, it is MORE efficient to use 'TryEnterCriticalSection' once you're inside a CS than to keep using 'EnterCriticalSection', but maybe you didn't know that.

What I WILL say to you directly, is, KISS OFF!!!
0
 
LVL 11

Expert Comment

by:mikeblas
ID: 2633000
try> Besides, I don't see where you had mentioned anything to him about 'TryEnterCriticalSection',
 try> which is integral and just as important when working with Critical Sections, as
 try> 'EnterCriticalSection'.  

It is?  How can that be?

TryEnterCriticalSection() isn't supported under Windows 95, or Windows 98. Or Windows NT 3.51. Meanwhile, EnterCriticalSection() is supported under all of those operating systems.

But those operating systems still fully support writing multithreaded applications, even if those applications use critical sections.

So, how could TryEnterCriticalSection() be so germaine to working with critical sections, but not supported on the bulk of today's installed Windows operating environments?

In other words, try, your assertion makes no sense!

..B ekiM
0
 
LVL 3

Expert Comment

by:Try
ID: 2634169
TryEnterCriticalSection() isn't supported under Windows 95, or Windows 98. Or Windows NT 3.51. Meanwhile, EnterCriticalSection() is supported under all of those operating systems.

-----------------------------------

Where has "airek" stated his platform is Win 95/98, or Win NT 3.51.  Why do you PRESUME it's one of those?  I can just as easily make a case that 'TryEnterCriticalSection' is supported under Win NT 4.0 and Win 2000, and that all the other API's relevant to Critical Sections are suported under those platforms as well, ... and is germaine to them.

If Microsoft had meant for Win 95/98 and Win NT 3.51 to be its platform mainstay, it wouldn't have developed Win NT 4.0 and Win 2000.  (Does THAT make sense?)

Before you start running off telling people what makes sense or doesn't make sense, I would do some hard introspection if I were you.
0
 
LVL 32

Expert Comment

by:jhance
ID: 2634277
Microsoft's internal reasons for doing things aside, I'd still like to hear from airek on my comment.  If you want to debate what Microsoft's goals for Win9x family products vs. NT products, why don't you post a question of your own.  I'm sure it would be interesting.


To recap my point....

About the only thing I can see here that would stop the acquisition thread is the EnterCriticalSection call.

Since you've only posted an excerpt of your code, my first suspicion is that you are calling EnterCriticalSection in your display thread and somehow missing a paired call to LeaveCriticalSection.  After that happens, the display thread continues to have the lock on the critical section and the acquisition thread is forever locked out.

airek,

What about the rest of your code?
0
 
LVL 3

Expert Comment

by:Try
ID: 2634544
"airek", I am convinced you are having this problem NOT because you don't know sufficiently about the workings of Critical Sections.  This problem is occurring due to a design issue, and without the benefit of seeing more of your code, the only thing I would suggest (if you're not under a tight deadline) is for you to re-think the way your logic is set out and try to rid your application of any unnecessary compilcated pieces of logic.  This is where my earlier advice on simplicity comes in.

I am also convinced this is not a really tough case to solve.  Just clear your mind and walk though things methodically and I'm sure you'll find the problem:  be it an unpaired 'Enter/LeaveCriticalSection', an uninitialized value, or something else.  I'm sure you could find it on your own.

If, after giving the situation your best shot and you still come up empty, post some extra code for us to see and I can guarantee you, a solution will be forthcoming.

I would like to say (to you) that I'm NOT interested in your points!  OK!  I'm one hundred percent interested in a solution to your technical problem, and if that solution comes as a result of something I might say (or have mentioned), keep your points and return the favor to someone else.

BTW, what operating system are you using?
0
 

Author Comment

by:airek
ID: 2636682
The application aquires image data from an external device through a  National Instruments data aquisition card (DIO6533) at a rate of 30 frames per second and displays it. The data is in 24 bit RGB format. The parameter passed to ReadThread is the address of a BOOL to indicate whether to stop. the parameter passed to DisplayThread is a struct containing the window handle and the same BOOL passed to ReadThread. Platform is Win 98, Visual Studio 6. Application is MFC dialog based. I placed counters in each thread for debugging and after about 200 cycles the ReadThread stops while Display Thread keeps running.   Here is the data acquisition thread function:
UINT ReadThread(LPVOID pParam)
{
      BOOL* pContinue = (BOOL*)pParam;
      i16 iDevice = 1;
    i16 iGroup = 1;

    i16 iStatus = 0;
    i16 iRetVal = 0;
    i16 iGroupSize = 4;
    i16 iPort = 0;
    i16 iDir = 0;
    i16 iSignal = 3;
    i16 iEdge = 0;
    i16 iReqPol = 0;
    i16 iAckPol = 0;
    i16 iAckDelayTime = 0;//50 ns

    u32 ulCount = BUFFERSIZE;
    u32 ulRemaining = 1;
    i16 iIgnoreWarning = 0;
    i16 iYieldON = 1;
      i16 iResource = 13;
      u32 puAlignIndex = 0;
      int Counter=0;
      
    // Configure group of ports as input, with handshaking.
      
    iStatus = DIG_Grp_Config(iDevice, iGroup, iGroupSize, iPort,
            iDir);
      
    iRetVal = NIDAQErrorHandler(iStatus, "DIG_Grp_Config",
            iIgnoreWarning);
      
    // Configure handshaking parameters for burst mode handshaking
   
      
    iStatus = DIG_Grp_Mode(iDevice, iGroup, iSignal, iEdge, iReqPol,
            iAckPol, iAckDelayTime);
      
    iRetVal = NIDAQErrorHandler(iStatus, "DIG_Grp_Mode",
            iIgnoreWarning);
      
      
            // You may want to call 'Align_DMA_Buffer' at this point if you
      //      have a buffer larger than 4kBytes in size and if your handshaking
//      signals occur at intervals faster than 50us.
    iStatus = Align_DMA_Buffer(iDevice, iResource, piBuffer1, ulCount,
            ulCount, &puAlignIndex);
      
    iRetVal = NIDAQErrorHandler(iStatus, "Align_DMA_Buffer",
            iIgnoreWarning);
      do
      {
            while(1)
            {
                  EnterCriticalSection(&g_CritSec1);
                  if(BufferState == FULL)
                  {
                        LeaveCriticalSection(&g_CritSec1);
                        WaitForSingleObject(g_NotFullEvent,INFINITE);
                        continue;
                  }
                  break;
            }

            // Start the handshaked buffered input of BUFFERSIZE "items".
            ulRemaining = 1;       

            iStatus = DIG_Block_In(iDevice, iGroup, piBuffer1, ulCount);
            
            iRetVal = NIDAQErrorHandler(iStatus, "DIG_Block_In",
                  iIgnoreWarning);
            
            //Apply your handshaking signals to the appropriate handshaking I/O pins.
            while ((ulRemaining != 0) && (iStatus == 0))
            {            
                  iStatus = DIG_Block_Check(iDevice, iGroup, &ulRemaining);      
                  iRetVal = NIDAQYield(iYieldON);
            }
            iRetVal = NIDAQErrorHandler(iStatus, "DIG_Block_Check",
                  iIgnoreWarning);
            BufferState = FULL;            
            LeaveCriticalSection(&g_CritSec1);
            PulseEvent(g_NotEmptyEvent);
      }while(*pContinue);
      
    // CLEANUP - Don't check for errors on purpose.
      
    // Clear the block operation.
      
   iStatus = DIG_Block_Clear(iDevice, iGroup);
      
    // Unconfigure group.
      
    iStatus = DIG_Grp_Config(iDevice, iGroup, 0, 0, 0);

      return 0;

}

Here is the display thread:
UINT Display::ShowImageThread(LPVOID pParam)
{
      unsigned int Counter = 0;
      CString str;
      DispParms* pDispParms = (DispParms*) pParam;
      //get screen dc and FreeRun
      BOOL* pContinue = pDispParms->pContinue;

      HDC pDC = ::GetDC(pDispParms->WinHandle);
      //create a memory dc
      HDC MemDc = ::CreateCompatibleDC(pDC);
      HBITMAP ImageBmp;
      do{
            while(1)
            {
                  EnterCriticalSection(&g_CritSec1);
                  if(BufferState == EMPTY)
                  {
                        LeaveCriticalSection(&g_CritSec1);
                        WaitForSingleObject(g_NotEmptyEvent,INFINITE);
                        continue;
                  }
                  break;
            }
            ImageBmp = CreateBitmap(WIDTH, HEIGHT, 1, 24, (unsigned char*)piBuffer1);

            //select the bitmap into the memory dc
            ::SelectObject(MemDc,ImageBmp);

            //blit to screen
            ::BitBlt(pDC,0, 0 ,WIDTH,HEIGHT, MemDc, 0, 0, SRCCOPY );
            
            DeleteObject(ImageBmp);
            BufferState = EMPTY;
            LeaveCriticalSection(&g_CritSec1);
            PulseEvent(g_NotFullEvent);

      }while(*pContinue);

      ::DeleteDC(MemDc);
      return(1);
}
 
0
 

Author Comment

by:airek
ID: 2636728
Here is the global function that initializes the sync objects:
void InitSyncObjects()
{

      InitializeCriticalSection(&g_CritSec1);
      g_NotEmptyEvent = CreateEvent(NULL,TRUE,FALSE,NULL);
      g_NotFullEvent = CreateEvent(NULL,TRUE,FALSE,NULL);
      BufferState = EMPTY;
}
0
 
LVL 32

Expert Comment

by:jhance
ID: 2636746
You've still left an important question unanswered.

Where does the acquisition thread get stopped?
0
 
LVL 1

Expert Comment

by:tomkeane
ID: 2638153
You are creating Manual Reset events.  Have you tried using Automatic Reset events instead?  To do this, make the following changes:

replace:
CreateEvent( NULL, TRUE, FALSE, NULL );
with:
CreateEvent( NULL, FALSE, FALSE, NULL );

and

PulseEvent()
with
SetEvent()

0
 
LVL 3

Expert Comment

by:Try
ID: 2638206
AHA!  I think, I've found something!

Just as suspected, you have an unbalanced 'Enter/LeaveCriticalSection'.

Let's look at your data acquisition code.

You're in a 'do/while' loop, and you'll stay in that loop until either a 'break' statement is issued (which there isn't any for this loop), or the while condition is satisfied (which is the question that "jhance" was asking, "where, in effect, is the 'while' condition [for the 'do/while'] gets turned off?").  IOW, where is '*pContinue' getting set to zero or FALSE (since it's a BOOL)?

But let's look at the 'Enter/LeaveCriticalSection' imbalance you're accumulating in the meantime.

-----------------------------

while(1)
{
EnterCriticalSection(&g_CritSec1);
if(BufferState == FULL)
{
LeaveCriticalSection(&g_CritSec1);
WaitForSingleObject(g_NotFullEvent,INFINITE);
continue;
}
break;
}

-------------------------------

When you enter that loop, the first thing you do, is enter a CS.  If 'BufferState' is NOT FULL, you get out of the 'while(1)' statement immediately, leaving the CS imbalanced, and hoping that somewhere along the way you'll come across a 'LeaveCriticalSection' to balance it out.

The next 'LeaveCriticalSection' appears just before you approach the exit of the 'do/while' statement.  OK, so now you have a balance to 'EnterCriticalSection', and everything is allright so far even if you don't re-enter the 'do/while' statement.

But what happens if you re-enter the 'do/while' statement?  When you're in the 'while(1)' loop and 'BufferState' is FULL, what then?  You enter into that segment of processing and the first thing you do, is balance out the 'EnterCriticalSection', and couple statements later, you exit from that segment (using the 'continue' statement) but immediately you evaluate the condition again.  So again you re-enter the segment and again you do another 'LeaveCriticalSection' which starts creating imbalances.  It's not until that condition (somehow) turns false that you exit from the 'while(1)' statement, ... WITH A SEVERELY IMBALANCED 'Enter/LeaveCriticalSection' pair.  However, later on (just before you exit the 'do/while' statement, you execute another 'LeaveCriticalSection' which causes more imbalances, which will keep worsening until '*pContinue' gets turned off (which by the time that happens, nobody knows how many times you may have gone through the 'do/while' loop).

That's my first take on what I see.

Remember, a 'continue' statement causes the current iteration of the nearest enclosing loop statement to terminate.  However, IT DOES NOT TERMINATE THE LOOP.  It only gets you out of the loop temporarily, but swings you right around to cause execution to resume with the evaluation of the same condition again, ... in the same loop.

A 'break' statement terminates the nearest enclosing 'while', 'do/while', 'for', or 'switch' statement.
0
 
LVL 1

Accepted Solution

by:
tomkeane earned 200 total points
ID: 2638323
Try: I don't see the unbalanced calls to Enter\Leave Critical section.  Each time through the while loop, EnterCriticalSection will be called onnce - then one of two things will happen, either the body of the if statement will be executed, in which case LeaveCS is called, followed by Wait... followed by the while loop executing again.
OR, the body of the if statement is not executed, and the codee breaks out of the while loop with the CS having a lock count of 1.  The code then acquires data and Leaves the CS, bringing the lock count back down to 0.

airek: Have you determined at what point in the Read data function the thread is 'stopped' at?  You can do this by running the program until it gets into the error state, then breaking the execution (Debug->Break).  If you then select Debug->Threads... you will get a list of threads runnning in your app.  Presumably one of these will be the Read data thread.  If you double click on it, you should be able to see where in the function it is hanging, and from there get a better idea of what to concentrate on: If it is stopped in a call to EnterCriticalSection, then you have an unbalanced call somewhere, if it is stopped in WaitForSingleObject() then you have a problem with the Events.  Other possibilities are that it is stopped somewhere in the NIxxx data acqusition code or that the thread exited prematurely for some reason.

HTH-
Tom


0
How your wiki can always stay up-to-date

Quip doubles as a “living” wiki and a project management tool that evolves with your organization. As you finish projects in Quip, the work remains, easily accessible to all team members, new and old.
- Increase transparency
- Onboard new hires faster
- Access from mobile/offline

 
LVL 11

Expert Comment

by:mikeblas
ID: 2638335
Try> Where has "airek" stated his platform is Win 95/98, or Win NT 3.51.

aierk's platform is irrelevant to your point.

 > Have you determined at what point in the Read data function the thread is 'stopped' at?  

This is a very important bit of information.

..B ekiM
0
 
LVL 3

Expert Comment

by:Try
ID: 2639162
You were the one who made platform an issue "mikeblas"; not me.  You stated that 'TryEnterCriticalSection' was not relevant to Win 95/98 and Win NT 3.51, and that my suggestion of its (possible) use made no sense.

Because no one at that time had known what "airek's" platform was, it could not be discounted that it was NOT Win NT 4.0 or Win 2000.  (That issue was not cleared up until much later [like a day later]).  If it had turned out to be Win NT 4.0 or Win 2000, then 'TryEnterCriticalSection' WOULD BE RELEVANT to that platform.

As far as 'TryEnterCriticalSection' being relevant to "airek's" problem (and I didn't think I'd have to get into this, but since I have to "spoon feed" you, then let me do it, ... this once), 'TryEnterCriticalSection' was NOT offered as a solution!!!!!  It was offered as part of an explanation behind the workings of entering a CS, and re-entering it while still inside, without having to execute a 'LeaveCriticalSection'.  It was offered as a strategic option in the overall use of CS that is more efficient than having to enter a CS, leave the CS, then re-enter it again.

I didn't say anything about it was going to solve "airek's" problem.  I brought it up because "jhance" wanted to know what new point did my suggestion offered that his had not covered.  This has meaning when you consider no one had enough code to determine HOW the CS was being entered, exited, and re-entered again.

In doing analysis, nobody should ignore scrutinizing the process of HOW code is being executed, and if entering, exiting, and then re-entering could be the place where imbalancing was happening, then it is something that needs to be checked out, ... one way or the other.  The subsequent cleaning up of that logic by substituting a more effecient set of code, is what has relevance.

(Has this been explained to you in digestible form, or do I need to reduce it further?)

--------------------------------

"tomkeane", look closer!  (Maybe I'm suffering from tunnel vision after looking at this problem for so long, so tell me if I am missing out on something I am failing to see.)

---------------------------------

do
{

while(1)
{
EnterCriticalSection(&g_CritSec1);
if(BufferState == FULL)
{
LeaveCriticalSection(&g_CritSec1);
WaitForSingleObject(g_NotFullEvent,INFINITE);
continue;
}
break;
}

....

BufferState = FULL;
LeaveCriticalSection(&g_CritSec1);
PulseEvent(g_NotEmptyEvent);
}while(*pContinue);

-------------------------------

"airek" enters the 'while(1)' loop statement, and immediately enters a CS.  If the 'BufferState' test fails, he hits 'break' and exits the 'while(1)' loop.  Later on, just before coming to the end of the 'do/while' statement, he balances out the 'EnterCS' with the 'LeaveCS' statement; no problem so far!

Notice, however, one of the things he does before executing the 'LeaveCS' statement (near the end of the 'do/while' loop) is to set 'BufferState' to FULL, so that the next time through the 'do/while' when he reaches the 'while(1)' loop, he will enter another CS, satisfy the BufferState test, balance out the 'EnterCS' (he will have just entered in with the 'LeaveCS'), BUT THE 'CONTINUE' STATEMENT WILL SWING HIM RIGHT BACK UP TO 'while(1)' where he will enter yet another CS, pass the 'BufferState' test, balance the CS with the 'LeaveCS', do the 'Wait' thing and then the 'continue' statement will swing him right back up again to the 'while(1)' loop.

It looks to me that he will NEVER leave the 'while(1)' loop as long as he keeps satisfying the 'BufferState' test.

He will have finished with the acquisition thread (for sure), but he will not be out of the loop and NEITHER WILL HE BE OUT OF THE 'do/while' loop.

This seems to fit with "airek's" original statement, "this runs for a while then the data acquisition thread stops and the display thread keeps running."

My recommendation at this time is to remove both the 'continue' statement and the 'LeaveCS' statement from inside the 'if' statement, because 'WaitForSingleObject' will wait until it has been signaled, for which there is no need to go back up to the 'while(1)' statement.  He will then fall through after executing the 'break' statement and complete the balancing of the CS.

We still need to know WHERE he turns off the 'do/while' condition.
0
 
LVL 1

Expert Comment

by:tomkeane
ID: 2639287
Try: I think you must be suffering from that tunnel vision you mentioned. ;-)

---------------------------------
1: do
2: {
3:  
4: while(1)
5: {
6: EnterCriticalSection(&g_CritSec1);
7: if(BufferState == FULL)
8: {
9: LeaveCriticalSection(&g_CritSec1);
10: WaitForSingleObject(g_NotFullEvent,INFINITE);
11: continue;
12: }
13: break;
14: }

15: ....

16: BufferState = FULL;
17: LeaveCriticalSection(&g_CritSec1);
18: PulseEvent(g_NotEmptyEvent);
19: }while(*pContinue);

-------------------------------

>"airek" enters the 'while(1)' loop statement, and immediately enters a CS.  If the >'BufferState' test fails, he hits 'break' and exits the 'while(1)' loop.  Later on, just >before coming to the end of the 'do/while' statement, he balances out the 'EnterCS' with the >'LeaveCS' statement; no problem so far!
It sounds like we agree that if the body of the if statement never executes things are peachy.   It's you analysis of what happens when we go into the body of the if statement that is flawed.

Let's trace which lines of code will be executed in the case where (BufferState == FULL).  I've numbered the lines to make this clearer.  Starting from while(1) (line 4) we have:

(6) EnterCS() - take the lock
(9) LeaveCS() - release the lock
(10) WaitForSingleObject() - wait for the event
(11) // continue...
(6) EnterCS() - take the lock
(9) LeaveCS() - release the lock
(10) WaitForSingleObject() - wait for the event
(11) // continue...
.... at some point BufferState becomes !FULL, so we have
(6) EnterCS() - take the lock
(13) break;
... data acquisition code
(17) LeaveCS() - release the lock
(19) while (*pContinue)
.... start all over again
(6) EnterCS() - take the lock
(9) LeaveCS() - release the lock
(10) WaitForSingleObject() - wait for the event
(11) // continue...

I don't care how many times that loop executes, the calls to EnterCS & LeaveCS will be balanced.  In fact the loop will execute at most once each time, since g_NotEmptyEvent will not be set (thus releasing thread 1 from the WaitForSingleObject call) until thread 2 has set  BufferState to EMPTY, but this is irrelevant, since the calls are balanced anyway.

>My recommendation at this time is to remove both the 'continue' statement and the 'LeaveCS' >statement from inside the 'if' statement, because 'WaitForSingleObject' will wait until it has >been signaled, for which there is no need to go back up to the 'while(1)' statement.

Now what would happen if the call to LeaveCS was removed from the body of the if statement?
DeadLock!  Here's why.  Thread 1 Enters the critical section, then immediately waits on the event to be set.  Meanwhile thread 2, the one who is responsible for setting said event, also tries to Enter the critical section, which it can't do, because thread one currently owns it.  So we have a classic deadlock situation, thread 1 is waiting on something from thread 2, which is waiting on something from thread 1, ad-inifinitum, ad-naseum (and ad-lackOfSleepUm...)

0
 
LVL 3

Expert Comment

by:Try
ID: 2642652
"tomkeane" you make a very good case, and on the surface you have me wanting to believe you (and you have succeeded partially), but I am not yet ready to come fully aboard, and here's why.

The first time the user comes through the 'do/while' loop, it finds BufferState "EMPTY".  So it fails the test the first time and falls through towards the 'do/while' loop's exit.  But, as I pointed out earlier, before swinging back to the beginning of that loop, it sets BufferState to "FULL", which means when it hits the 'while(1)' loop, and CS is entered, the 'if' statement will succeed and it will exit the CS (by way of 'LeaveCS').  There it will sit and wait for the object (which is an event in this case) to be signaled.

Waiting, ... waiting, ... waiting.

Finally, the event is signaled, and what does it see next?  The 'continue' statement, which tells it to loop back up to the 'while(1)' statement (which it does) and so it enters a the CS once again, and "Lo and Behold" BufferState is still "FULL".  It was NOT turned off when it was previously inside the 'if' statement.

"airek's" code did not turn BufferState off, therefore it's going to keep looping round and around in that 'while(1)' loop (and in your words), "ad-inifinitum, ad-naseum (and ad-lackOfSleepUm...)".

What is interesting to note here, is that while activity is taking place between the 'while(1)' statement and the success of entering the 'if' statement, there is no imbalancing of CS occurring.  However, following the instruction of that 'continue' statement, when it loops back and does not find BufferState set to "EMPTY", there will be, "ad-inifinitum, ad-naseum (and ad-lackOfSleepUm...)".

Show me where once inside the 'if' statement BufferState is turned off (preventing it from re-entering it again), and I will go along with the idea that the data acquisition thread is functioning OK.  (Just "OK", because I still need to see where '*pContinue' is being turned off, to assure a clean exit of the 'do/while' loop.)
0
 
LVL 3

Expert Comment

by:Try
ID: 2642812
I would like to clarify the point that I seek no credit, by some means of indirection, that the idea originated with me to ask, "Where does the acquisition thread get stopped?"  That was "jhance's" question and is not to be construed as some sort of earthshaking realization that I came into lately.  He originated the question and whatever credit is due to its insight, should go to him.

The question, however, still remains central and in need of answering.

0
 
LVL 1

Expert Comment

by:tomkeane
ID: 2643156
Try:
Show me where once inside the 'if' statement BufferState is turned off (preventing it from re-entering it again), and I will go along with the idea that the data acquisition thread is functioning OK.  (Just "OK", because I still need to see where '*pContinue' is being turned off, to assure a clean exit of the 'do/while' loop.)

Thread2 (the display thread) sets BufferState=EMPTY just before it calls LeaveCS followed by PulseEvent().

This is a classic Producer/Consumer architecture, with the Data Acquisition thread as the producer and the Display thread as the consumer.  
The producer gets some data, places it in a shared slot, signals the waiting consumer, and then waits for the slot to be empty.  The consumer wakes up, grabs the thing out of the shared slot, does something with it, signals the waiting producer thread, and then waits for the slot to be filled again.

They both do this until some external entity (presumably the host application) sets a flag (*pContinue, which was passed in with the thread creation parameters) signalling them both that they should quit.

The book 'Multi-threaded Programming with Windows NT' (Pham & Garg) has a good treatment of this topic in chapter 3.

I do agree, however, that the most important piece of information here is missing, which is where exactly the Data Acquisition thread stops.
0
 
LVL 3

Expert Comment

by:Try
ID: 2644864
Maybe "airek" does not need the 'do/while' statement.  The example on page 57 & 58 in "Multi-threaded Programming with Win NT", by Pham & Garg, simply uses the 'PulseEvent' API to switch back and forth between threads (with one alerting the other), while 'WaitForMultipleObjects' simply waits (using "TRUE" and "INFINITE") until all the threads are finished with their work.

Why is there a need for the 'do/while' statement, for both threads?  (The question is really rhetorical because unless I see the code, I am just asking it in a manner similar to thinking out loud.)

Ordinarily, the condition governing the 'while' potion of the 'do/while' statement, is turned off (directly or indirectly) inside the body of the enclosing braces, so that when the last statement is reached, the 'while' portion will know whether it should swing back up to the 'do' portion of the loop.

If you must, keep the 'do/while', but put a condition INSIDE its body that will instruct the 'while' portion when not to repeat the loop.
0
 

Author Comment

by:airek
ID: 2645267
My apologies for an imperfect understanding of what was going on within my program. Both threads continue to run but the buffer stops being filled by the acquisition thread so the image displayed freezes. The do-while statement is broken by an external variable that is set in the main thread this allows me to tell the program to stop aquiring data. It must execute at least once. The synchronization method is sound and works as expected. If DisplayThread executes before the first DataThread it waits until the notempty event to be set by the data thread. In this way the threads are forced to run sequentially Aquire/Display Aquire/Display. The problem was with the display routine. When I called ::SelectObject(MemDc,ImageBmp); i created a memory leak by not selecting the previous object into the device context. By using the Sstem monitor and veiwing allocated memory as the program ran memory ramped up until there was no more memory to allocate for the aquisition operation and the data stops being acquired. This was replaced with            oldBitmap=(HBITMAP)::SelectObject(MemDc,ImageBmp);
            //blit to screen
            ::BitBlt(pDC,0, 0 ,WIDTH,HEIGHT, MemDc, 0, 0, SRCCOPY );
            ::SelectObject(MemDc,oldBitmap);
Allocated memory remains flat and all function as expected. Thank you all for the valuable you've spent on this problem. tomkeane gave me the best troubleshooting advice as I was unfamiliar with debugging multithreaded apps and didn't know about the Debug->Threads menu command so he gets the points.
0
 
LVL 3

Expert Comment

by:Try
ID: 2646773
One more thing I've learnt:  Not to get too consumed with one aspect of a program that you ignore other areas of it.  One more lesson for me!

I was too focus and caught up in the CS aspect of the problem, that I did not pay any attention to the other posible ailments of it.

Good work "tomkeane"!   Good work "airek"!
0
 
LVL 11

Expert Comment

by:mikeblas
ID: 2647615
Try> That was "jhance's" question and

Uh, no. That was what I asked in my first post, which was the very first post in response to this question.

Knowing where the thread stops is crucial to diagnosing the problem. That _shouldn't_ be a great revelation, but most people aren't very good debuggers. And the answer to that question still seems to have escaped this thread.

..B ekiM
0
 
LVL 11

Expert Comment

by:mikeblas
ID: 2647622
> My apologies for an imperfect understanding of what was going on within my program.

Debugging is a waning art, I'm afraid.

But I'm not sure why you didn't ask--I queried about the state of your threads immediately. If you didn't know how to check them, you should have asked and I would have been happy to explain it.

I'm glad you've got your problem solved.

..B ekiM
0
 
LVL 3

Expert Comment

by:Try
ID: 2647681
There is a big difference between asking a question, ... and asking what a question means.

"jhance" asked the question!!!!

Here it is:   Where does the acquisition thread get stopped?

----------------------------

You didn't ask the question; you asked what the question meant!!!!

Here is it:   What does "the data acuqisition thread stops" mean?

-----------------------------

Asking what a question means, is NOT asking the question.  It is asking for further enlightenment OF the question.
0
 
LVL 11

Expert Comment

by:mikeblas
ID: 2648027
It's really inconsequential, but please read my first posting.  I wrote, there: "If you break into the debugger while your thread is starved, where is the statement pointer?"

But I'm wondering why you seem to have such a chip on your shoulders. I've not before seen someone get so agitated, abrasive, and demeaning in one question. Is there some past history between you and some of the others in this thread, or something?

..B ekiM
0
 
LVL 3

Expert Comment

by:Try
ID: 2648368
This matter is CLOSED.
0
 
LVL 11

Expert Comment

by:mikeblas
ID: 2649884
> This matter is CLOSED.

Maybe as far as you're concerned. But I'm still chock full o' questions.

..B ekiM
0

Featured Post

How your wiki can always stay up-to-date

Quip doubles as a “living” wiki and a project management tool that evolves with your organization. As you finish projects in Quip, the work remains, easily accessible to all team members, new and old.
- Increase transparency
- Onboard new hires faster
- Access from mobile/offline

Join & Write a Comment

Introduction: Dynamic window placements and drawing on a form, simple usage of windows registry as a storage place for information. Continuing from the first article about sudoku.  There we have designed the application and put a lot of user int…
Introduction: The undo support, implementing a stack. Continuing from the eigth article about sudoku.   We need a mechanism to keep track of the digits entered so as to implement an undo mechanism.  This should be a ‘Last In First Out’ collec…
This video will show you how to get GIT to work in Eclipse.   It will walk you through how to install the EGit plugin in eclipse and how to checkout an existing repository.
Access reports are powerful and flexible. Learn how to create a query and then a grouped report using the wizard. Modify the report design after the wizard is done to make it look better. There will be another video to explain how to put the final p…

760 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

17 Experts available now in Live!

Get 1:1 Help Now