Solved

Simple multi-threading synchronization question

Posted on 1998-08-12
17
238 Views
Last Modified: 2013-11-20
Hi,

I am working on my first multi-threaded program. It generates a star field on the screen which is continually moving around; tough to explain.

Anyway, I thought that I would create a separate thread for the drawing of the stars. The main program thread would perform all of the calculations for where the stars are moving to.

So, when drawing, it leaves garbage behind. I know I am doing it right because I have done many programs like this (i.e. erase prev star, draw new star, calculate new pos, loop), and the problem hasn't started occurring until I tried multi-threading.

What I think is happening is the stars coordinates are being updated at the same time they're being drawn, which is kind of what I want (i.e. I want the program to go faster :-)).

I think I can solve the problem using something like CSemaphore, CMutex, or CCriticalLock and the CSingleLock classes, but I don't understand how to use them.

Here is a large part of the source code:

void CSpaceSaverDlg::OnTimer(UINT nIDEvent)
{ int i;
  int j = 0;

  dat->s = stars;
  dat->num = m_Number;
  dat->wnd = this;

  AfxBeginThread (ThreadProc, dat); // Passing global variable?  That's life.
  // Sleep (50); // Fixes some
  // Sleep (100); // Fixes all the way.

  if (rand () % 1000 == 0)
  { m_Direction = rand () % 8; }

  for (i = 0; i < m_Number; i++)
  { CSingleLock sLock (&stars[i].m, FALSE);
    if (sLock.Lock () == 0)
    { MessageBox ("Error");
      break;
    }

    stars[i].px = stars[i].x;
    stars[i].py = stars[i].y;

    switch (m_Direction)
    { case (0): stars[i].x += stars[i].z / m_Speed;
                break;

      case (1): stars[i].x += stars[i].z / m_Speed;
                stars[i].y += stars[i].z / m_Speed;
                break;

      case (2): stars[i].y += stars[i].z / m_Speed;
                break;

      case (3): stars[i].x -= stars[i].z / m_Speed;
                stars[i].y += stars[i].z / m_Speed;
                break;

      case (4): stars[i].x -= stars[i].z / m_Speed;
                break;

      case (5): stars[i].x -= stars[i].z / m_Speed;
                stars[i].y -= stars[i].z / m_Speed;
                break;

      case (6): stars[i].y -= stars[i].z / m_Speed;
                break;

      case (7): stars[i].y -= stars[i].z / m_Speed;
                stars[i].x += stars[i].z / m_Speed;
                break;
    }

    if (stars[i].x > m_MaxX)
    { stars[i].x -= m_MaxX; }
    else if (stars[i].x < 0.0)
    { stars[i].x += m_MaxX; }

    if (stars[i].y > m_MaxY)
    { stars[i].y -= m_MaxY; }
    else if (stars[i].y < 0.0)
    { stars[i].y += m_MaxY; }
    sLock.Unlock ();
  }

  CDialog::OnTimer(nIDEvent);
}

UINT ThreadProc (LPVOID pParam)
{ Data* dat = (Data *) pParam;

  if (dat == NULL)
  { return -1; }   // illegal parameter

  CDC *dc = dat->wnd->GetDC ();
  CBrush blankbrush (RGB (0, 0, 0));
  CBrush brush;
  int i;

  int m_Number = dat->num;
  Star *stars = dat->s;

  for (i = 0; i < m_Number; i++)
  { CSingleLock sLock (&stars[i].m, FALSE);
    if (sLock.Lock () == 0)
    { dat->wnd->MessageBox ("Error");
      break;
    }

    brush.CreateSolidBrush (stars[i].color);

    dc->SelectObject (blankbrush);
    dc->Ellipse ((int) stars[i].px - 2, (int) stars[i].py - 2, (int) stars[i].px + 2, (int) stars[i].py + 2);
    dc->SelectStockObject (NULL_BRUSH);
    //dc->SetPixel ((int) stars[i].px, (int) stars[i].py, RGB (0, 0, 0));

    dc->SelectObject (brush);
    dc->Ellipse ((int) stars[i].x - 2, (int) stars[i].y - 2, (int) stars[i].x + 2, (int) stars[i].y + 2);
    dc->SelectStockObject (NULL_BRUSH);
    //dc->SetPixel ((int) stars[i].x, (int) stars[i].y, stars[i].color);

    brush.DeleteObject ();
    sLock.Unlock ();
  }

  blankbrush.DeleteObject ();

  return 0;    // thread completed successfully
}

/////////////////////////////
// Here is the stars structure:

#ifndef STARH
#define STARH

#include <afxmt.h>

class Star
{ public:
  void Init (double, double);

  volatile double x, y, z;
  volatile double px, py;
  volatile COLORREF color;

  CMutex m;
};

#endif

// And the stars.cpp implementation:
#include "stdafx.h"
#include "star.h"

void Star::Init (double MaxX, double MaxY)
{ x = (double) (rand () % (int) MaxX);
  y = (double) (rand () % (int) MaxY);
  z = (double) (rand () % (int) 384 + 128.0);
  px = 0.0;
  py = 0.0;

  color = RGB ((int) z / 2, (int) z / 2, (int) z / 2);

  //color = RGB (255, 255, 255);
  return;
}


What am I doing wrong?  Why is garbage still being left behind?

Thank you for any help you can provide.  Note that if the solution is more complex than it seems, I can increase the number of points.
0
Comment
Question by:thresher_shark
  • 12
  • 5
17 Comments
 
LVL 6

Author Comment

by:thresher_shark
Comment Utility
Edited text of question
0
 
LVL 2

Accepted Solution

by:
SamratAshok earned 100 total points
Comment Utility
One of the things missing from your code is name of the Mutex object.
Each mutex needs to be named uniquely as pointing to a particular star. So obvious name choice is some fixed string concatenated by arrayindex. That could be the cause of whole garbage problem.

You will have to do it at the time of construction of mutex object, so looks like you will have to create mutexes objects all over the heap.

Some other suggesstions (First one is most important )

* Avoid changing x,y,z several times, use a temporary value and assign at the end
when values are ready.

* Try adding a get/ set function for (x,y,z) parameters in the star class.

* Instead of setting and painting all stars in one thread, do a star per thread and
then maybe reduce the timer-slice. (you will need atleast one more global variable
to keep track of next star to be changed)

* Instead of using thread to repaint the screen, simple invalidate portion of screen (with hints ) which will generate WM_PAINT and move the painting over there.

Hope this helps :-)


0
 
LVL 6

Author Comment

by:thresher_shark
Comment Utility
Thank you for your ideas.  I hadn't thought about the name of the mutex.  How would you recommend I generate a unique name for every star?

I also see what you mean about adjusting the stars coordinates all the time, as opposed to your idea which would be better in this case.

Thanks for your help.
0
 
LVL 6

Author Comment

by:thresher_shark
Comment Utility
Shoot, forget the first paragraph of the above comment.  I didn't read your answer thoroughly :-)
0
 
LVL 6

Author Comment

by:thresher_shark
Comment Utility
Well, this is strange, after trying your suggestions, the problem still occurs.

I have named each mutex individually.  I then changed the calculating code so it uses temporary variables.  Here is the new calculating code:

void CSpaceSaverDlg::OnTimer(UINT nIDEvent)
{ int i;
  int j = 0;
  double xinc = 0.0, yinc = 0.0, ztemp = 0.0;

  dat->s = stars;
  dat->num = m_Number;
  dat->wnd = this;

  AfxBeginThread (ThreadProc, dat); // Passing global variable?

  // Sleep (50); // Fixes some
  // Sleep (100); // Fixes all the way.

  if (rand () % 1000 == 0)
  { m_Direction = rand () % 8; }

  for (i = 0; i < m_Number; i++)
  { CSingleLock sLock (stars[i].m, FALSE);
    if (sLock.Lock () == 0)
    { MessageBox ("Error");
      break;
    }

    xinc = stars[i].x;
    yinc = stars[i].y;
    ztemp = stars[i].z;

    stars[i].px = xinc;
    stars[i].py = yinc;

    switch (m_Direction)
    { case (0): xinc += ztemp / m_Speed;
                break;

      case (1): xinc += ztemp / m_Speed;
                yinc += ztemp / m_Speed;
                break;

      case (2): yinc += ztemp / m_Speed;
                break;

      case (3): xinc -= ztemp / m_Speed;
                yinc += ztemp / m_Speed;
                break;

      case (4): xinc -= ztemp / m_Speed;
                break;

      case (5): xinc -= ztemp / m_Speed;
                yinc -= ztemp / m_Speed;
                break;

      case (6): yinc -= ztemp / m_Speed;
                break;

      case (7): yinc -= ztemp / m_Speed;
                xinc += ztemp / m_Speed;
                break;
    }

    if (xinc > m_MaxX)
    { xinc -= m_MaxX; }
    else if (xinc < 0.0)
    { xinc += m_MaxX; }

    if (yinc > m_MaxY)
    { yinc -= m_MaxY; }
    else if (yinc < 0.0)
    { yinc += m_MaxY; }

    stars[i].x = xinc;
    stars[i].y = yinc;
    //stars[i].z = ztemp; // No changes to z so no need to change
    sLock.Unlock ();
  }

  CDialog::OnTimer(nIDEvent);
}

/////////////////////////////////
//
// I have not changed the drawing code at all.  Here is the new // star structure (for now):
//
/////////////////////////////////
/////// Stars.cpp:

#include "stdafx.h"
#include "star.h"

void Star::Init (double MaxX, double MaxY, int index)
{ x = (double) (rand () % (int) MaxX);
  y = (double) (rand () % (int) MaxY);
  z = (double) (rand () % (int) 384 + 128.0);
  px = 0.0;
  py = 0.0;

  color = RGB ((int) z / 2, (int) z / 2, (int) z / 2);
  //color = RGB (255, 255, 255);

  CString temp;
  temp.Format ("NameOfMutex%d", index);
  m = new CMutex (FALSE, temp);
  return;
}

Star::~Star ()
{ delete m;
}

///////////////////////////
// Stars.h

#ifndef STARH
#define STARH

#include <afxmt.h>

class Star
{ public:
  ~Star ();
  void Init (double, double, int);

  volatile double x, y, z;
  volatile double px, py;
  volatile COLORREF color;

  CMutex *m;
};

#endif


/////

But now for the really strange part.  If I un-rem the line in the Init function that sets the star's color to 255, 255, 255, it WORKS!  But otherwise it doesn't work!

It is probably something very simple, that I am just missing, but I don't know.

In any case, thank you very much for your help.
0
 
LVL 6

Author Comment

by:thresher_shark
Comment Utility
If you want, I can email you the whole project and you can see this "phenomenon" in action.
0
 
LVL 6

Author Comment

by:thresher_shark
Comment Utility
Here is some information I collected:

color = RGB (255, 255, 255); // Works
color = RGB (254, 254, 254); // Doesn't work
color = RGB (253, 253, 253); // Doesn't work
color = RGB (252, 252, 252); // Doesn't work

color = RGB (128, 128, 128); // Works
color = RGB (129, 129, 129); // Doesn't work

color = RGB (192, 192, 192); // Works

It seems things that have to do with a power of two work well.  But 255 doesn't have much to do with that except 2^8 - 1 = 255.  Anyway... I still don't understand.  Thanks for your help.
0
 
LVL 2

Expert Comment

by:SamratAshok
Comment Utility
This color thing is really strange. Send me the code if you can as an attachment.
My email address is cdjindia@bestweb.net

Also lemme know what compiler and what version are you using for this project.
 
0
What Should I Do With This Threat Intelligence?

Are you wondering if you actually need threat intelligence? The answer is yes. We explain the basics for creating useful threat intelligence.

 
LVL 6

Author Comment

by:thresher_shark
Comment Utility
I have sent the project.  I have Visual C++ with SP3.  I hope your mail reader understands the MIME format for email.
0
 
LVL 2

Expert Comment

by:SamratAshok
Comment Utility
I  have lost your email address. please send a fake mail either at
cdjindia@bestweb.net or cdjindia@hotmail.com and I can mail you the
new code.


0
 
LVL 6

Author Comment

by:thresher_shark
Comment Utility
I have sent you the message.
0
 
LVL 6

Author Comment

by:thresher_shark
Comment Utility
Have you sent the new code yet?  I am just dying to see what the problem is :-)
0
 
LVL 2

Expert Comment

by:SamratAshok
Comment Utility

Yes, I sent it already on Friday. Earliest I can send it again is now on Monday because I don't have that file at home machine. I sent it to your @gte account.


0
 
LVL 2

Expert Comment

by:SamratAshok
Comment Utility
On second thoughts (to keep you from dying ... :-; )

Implementation that you have for painting the stars is not actually as per microsoft API
specifications.
0
 
LVL 6

Author Comment

by:thresher_shark
Comment Utility
It might not have arrived yet because it's large, the mail servers probably have priorities and all kinds of other stuff to prevent themselves from getting too overloaded.
0
 
LVL 6

Author Comment

by:thresher_shark
Comment Utility
Hello again,

I found a small problem with my drawing code, I was not releasing the DC after I was done with it, however the garbage problem is still ocurring.  Can you please be more specific with what the problem was?  After all, anyone following this thread would probably like to know, especially if they had to pay for it :-)

Thank you so much for all of your help so far.
0
 
LVL 6

Author Comment

by:thresher_shark
Comment Utility
Adjusted points to 100
0

Featured Post

What Security Threats Are You Missing?

Enhance your security with threat intelligence from the web. Get trending threat insights on hackers, exploits, and suspicious IP addresses delivered to your inbox with our free Cyber Daily.

Join & Write a Comment

Suggested Solutions

Title # Comments Views Activity
ADO Memory leak with DELPHI 2007 37 153
iSeries FTP Exit Program 8 113
modThree challenge 4 64
Sed question 2 45
Introduction: Displaying information on the statusbar.   Continuing from the third article about sudoku.   Open the project in visual studio. Status bar – let’s display the timestamp there.  We need to get the timestamp from the document s…
Introduction: Dialogs (2) modeless dialog and a worker thread.  Handling data shared between threads.  Recursive functions. Continuing from the tenth article about sudoku.   Last article we worked with a modal dialog to help maintain informat…
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.
Polish reports in Access so they look terrific. Take yourself to another level. Equations, Back Color, Alternate Back Color. Write easy VBA Code. Tighten space to use less pages. Launch report from a menu, considering criteria only when it is filled…

744 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

16 Experts available now in Live!

Get 1:1 Help Now