[Last Call] Learn how to a build a cloud-first strategyRegister Now

x
?
Solved

Passing data to a thread

Posted on 2009-04-25
27
Medium Priority
?
644 Views
Last Modified: 2013-12-27
I have to pass data from my main program to a thread which stores it in a database.

So far I have used a CStringArray and have done this:
CStringArray sDataToStore;

When user click a button data is stored in the CStringArray like so:
sDataToStore.Add(sUserData);

then in the thread I access this data like so:

UINT CMyClass::MyThread(LPVOID lpObj)
{
     CMyClass *pobjDlg = (CMyClass*)lpObj;
     while(1)
     {
          for(int i=0; i< pobjDlg->sDataToStore.GetSize(); i++)
          {
               CString sData = pobjDlg->sDataToStore.GetAt(i);
               //store sData in Database blah blah
           }
     }
     return 0;
}

Is this the best/correct way to do this?

Thanks
     



}
0
Comment
Question by:Wanting2LearnMan
  • 13
  • 12
  • 2
27 Comments
 
LVL 40

Expert Comment

by:evilrix
ID: 24231642
2 observations....

1. You need to have mutex protection around any access to the close sDataToStore to ensure you don't have any race conditions... trying to write to it in 1 thread whilst reading from it in another.
http://en.wikipedia.org/wiki/Mutual_exclusion
http://en.wikipedia.org/wiki/Race_condition

2. You must declare sDataToStore to be volatile, otherwise the compiler is free to optimise away any changes, so your main thread may modify it but the other thread may not see the changes.
http://www.ddj.com/cpp/184403766

Really, what you want is a producer/consumer pattern.
http://en.wikipedia.org/wiki/Producer-consumer_problem
http://www.codeproject.com/KB/threads/ProducerConsumerModel.aspx
http://www.ddj.com/architect/184401751
http://www.penguin-soft.com/penguin/developer/j2setutorial/essential/threads/synchronization.html
0
 
LVL 40

Expert Comment

by:evilrix
ID: 24231973
Incidentally, I take it the above code is to prevent the UI from freezing whilst the database transaction is completed? This isn't going to help much since your Ul cannot update sDataStore any quicker than the thread can process it, otherwise you'll have a race condition (as noted above).

If your UI isn't expecting to do lots of updates then having a thread constantly alive to process the occasional click of a button probably isn't the best way to go. You're probably better of spinning up a new thread for each button click and pass in the data to the thread as it's started. This worker thread will then update the database and terminate. There is a slight overhead in the cost of starting a new thread but since the performance isn't critical (since you're limited by how quick a user can click buttons on the UI) it's not going to make any difference, but the overall solution will be a lot simpler to implement since you won't have to worry about mutual exclusion or race conditions.
0
 

Author Comment

by:Wanting2LearnMan
ID: 24233133
Yes the code is to prevent the UI from freezing.

The problem I face is that the user will be clicking the buttons very often. Maybe once a second or even faster.  I cannot have the UI slow down at all or the user will miss events they need to record and get frustrated.

I have noticed the problem you mentioned where the thread did not see changes I made to the array. I have just added the word volatile to the declaration of the CStringArray but I get errors like so:

error C2662: 'Add' 2 overloads have no legal conversion for 'this' poinnter
error C2662: 'GetSize' cannot convert 'this' pointer from volatile class CStringArray to const class CStringArray &
etc

What is the proper way to use the volatile, is it:
volatile CStringArray sDataToStore;  OR
CStringArray volatile sDataToStore;

Also
I am confused as to how to design this all properly.  What I want to do is to store the data in a database locally on PDA and also send it via wi fi to a server each time an event is reocorded.  So I will have the main UI thread, a thread for storing data in the database and a thread for taking data from the database and sending it.
I MUST not have the UI slow down any, thats why i have a database thread and a wifi thread.  I will design some sort of event mechanism between these two threads as they each have to access the database.  But now I think I also have to handle access to the sDataToStore variable between the UI and database thread.

Please help.

Thanks for your guidance.
0
Industry Leaders: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
LVL 40

Expert Comment

by:evilrix
ID: 24233613
>> The problem I face is that the user will be clicking the buttons very often. Maybe once a second or even faster
Then maybe you should consider a thread pool.
http://en.wikipedia.org/wiki/Thread_pool

>> CStringArray but I get errors like so:
The problem is related to the fact that because the class is volatile you can only call members on it that are declared volatile, much like if a class is const you can only call const members on it. This is part of the language specification, the only way to get around this is to write your own class or use a POD (plain old data) type.

Also note that defining a non-POD type in global space is a problem waiting to happen. All objects can throw exceptions on construction, and if an object throw in global space you can't catch it so the application will just crash.


>> What is the proper way to use the volatile, is it:
>> volatile CStringArray sDataToStore;  OR
>> CStringArray volatile sDataToStore;

Either, both means the same thing... same with const

int const i;
const int i;

are identical, I prefer the former form because it reads better when declaring const pointer....

char const * const

Always read a definition backwards... so that's a const pointer to a const char. But I digress :)

>> I am confused as to how to design this all properly.
Use a queue, protect access to it by a mutex. Create a thread pool and pass the address of the queue into each thread (then you don't need to declare it volatile because the object being pointed to is changing and not the pointer itself). When your user clicks a button push the request into the queue. Each thread in your pool just deques the next request in the queue and processes it until the queue is empty when they all sleep. You can use an event to signal the threads to release one when you pop something into the queue.






0
 
LVL 40

Accepted Solution

by:
evilrix earned 2000 total points
ID: 24233807
Below is a very simple example. It's by no means a complete bit of code and has only been tested quickly (so may contain defects). I won't got into how it works here because I've commented the code in detail and when you read it you should be able to make sense of what's going on. If you're not sure about the options I took when defining my mutex and/or events read up on them on the MSDN and I'm sure it'll be clear. If not then please ask and I'll do my best to clarify anything you don't get.
#include <windows.h>
#include <process.h>
 
#include <deque>
#include <vector>
#include <string>
#include <iostream>
#include <sstream>
 
// Typedefs just to aid readability
typedef std::deque<std::string> string_queue_t;
typedef std::vector<HANDLE> handle_vector_t;
 
// Thread data
struct thread_data
{
	string_queue_t string_queue;
	HANDLE mtx;
	HANDLE evts[2]; // 0 Quit, 1 Process
};
 
// A thread
void thread_proc(void * pv)
{
	// If we have no thread data terminate
	if(0 == pv) return;
 
	// Get the thread data
	thread_data & td = *reinterpret_cast<thread_data *>(pv);
 
	bool bQuit = 0;
	while(!bQuit)
	{
		// Wait for either a job or signal to quit
		DWORD result = WaitForMultipleObjects(2, td.evts, false, INFINITE);
 
		// Get exclusive access to the queue
		WaitForSingleObject(td.mtx, INFINITE);
 
		// Take a copy of the job in the queue
		// NB. working with a copy means we can free up access to the queue immediately
		std::string s;
		if(td.string_queue.size() > 0)
		{
			s = td.string_queue.front();
			td.string_queue.pop_front();
		}
		else
		{
			// The queue is empty so if we were signaled to quit then we can do so now
			bQuit = ((result - WAIT_OBJECT_0) == 0);
		}
 
		// Release exclusinve lock on queue
		ReleaseMutex(td.mtx);
 
		// Process what we got out of the queue
		// NB. Since this is a copy the queue has been released from exclusive access
		if(!s.empty())
		{
			// This output is thread safe but may end up mixing with other threads
			// that's ok it's just for example purposes.
			std::cout << "tid: " << GetCurrentThreadId() << " - " << s << std::endl;
		}
	}
}
 
 
int main()
{
	// Create out cross-thread object
	thread_data td;
 
	// Mutex to sync access to the queue
	td.mtx = CreateMutex(0, false, 0);
 
	// Event used to signal threads to terminate
	td.evts[0] = CreateEvent(0, true, false, 0);
 
	// Event used to signal the thread pool a job is ready
	td.evts[1] = CreateEvent(0, false, false, 0);
 
	// A vector that will represent the thread pool
	handle_vector_t handle_vector;
 
	// Create thread pool
	for(int i = 0 ; i < 10 ; ++i)
	{
		HANDLE handle = reinterpret_cast<HANDLE>(_beginthread(thread_proc, 0, &td));
		if(handle > 0)
		{
			handle_vector.push_back(handle);
		}
	}
 
	// Queue jobs for processing by thread pool
	for(int j = 0 ; j < 9999 ; ++j)
	{
		std::stringstream ss;
		ss << "data " << j;
 
		// Add to queue
		WaitForSingleObject(td.mtx, INFINITE);
		td.string_queue.push_back(ss.str());
		ReleaseMutex(td.mtx);
 
		// Signal the thread pool
		SetEvent(td.evts[1]);
	}
 
	// Signal threads to terminate when there are no more jobs
	SetEvent(td.evts[0]);
 
	// wait for threads to terminate
	WaitForMultipleObjects(3, &handle_vector[0], true, INFINITE);
}

Open in new window

0
 

Author Comment

by:Wanting2LearnMan
ID: 24237300
thanks very much for all this.  I'll take time now and go through it all.  I'll probably have more questions soon.

Thanks
0
 
LVL 40

Expert Comment

by:evilrix
ID: 24237311
>> thanks very much for all this.
You are very welcome

>> I'll take time now and go through it all.
Of course, take as much time as you need

>> I'll probably have more questions soon.
No worries... it's what we're here for :)
0
 

Author Comment

by:Wanting2LearnMan
ID: 24240881
Hate to annoy you with basic questions but my project is using eVC++4 (Its a PocketPC MFC Dialog based project).
When I add in the code above for testing I get errors.
e.g. #include <iostream>
fatal error C1083: Cannot open include file: 'iostream.h': No such file or directory
I read somewhere that the these files are not supported when using eVC++4.

e.g. when I add #include <deque> I get lots and lots of warnings:
warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify -GX

AmI snookered because im using eVC++4 or can the above be modified to suit??

Thanks


0
 
LVL 40

Expert Comment

by:evilrix
ID: 24240926
>> e.g. #include <iostream>
>> fatal error C1083: Cannot open include file: 'iostream.h': No such file or directory
>> I read somewhere that the these files are not supported when using eVC++4.

iostream.h is a deprecated header in C++; however, I presume you are including <iostream> which is part of the C++ standard but it's possible it's not supported by eVC++4. The use of iostream was only to demonstrate the principle, it's not an integral part of the solution. In fact the example was only intended to show you how to use a queue to do this. Try running it and stepping through it on a normal PC and when you are happy with the principle you can code up yourself an eVC++4 version.

>> e.g. when I add #include <deque> I get lots and lots of warnings:
>> warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify -GX
That means the code is htrowing C++ exception types but your project setting has stack unwiding disabled. The two are not compatible. You need to enable stack unwiding in your project. Again, the use of queue may not be supported by eVC++4, it should be treated just as an example and not a solution.

>> AmI snookered because im using eVC++4 or can the above be modified to suit??
Yes, but you'll probably have to use Microsoft specific contructs to do so such as CQueue (maybe)
http://msdn.microsoft.com/en-us/library/dd374994(VS.85).aspx
0
 

Author Comment

by:Wanting2LearnMan
ID: 24240976
thanks for quick response, I seem to hit walls every where I turn :(
 I will investigate queues in the eVC++4 world now
0
 
LVL 40

Expert Comment

by:evilrix
ID: 24241001
>> thanks for quick response, I seem to hit walls every where I turn :(
The principle (the design pattern) will be the same no matter what platform you're targeting but you'll have to choose the constructs specific to your platform. I chose to use STL containers because they are part of the C++ standard, it's unfortunately (and surprising) that Microsoft chose not to support STL in eVC++4.
0
 

Author Comment

by:Wanting2LearnMan
ID: 24241529
I found a STl library for eVC++ here:
http://www.syncdata.it/stlce/index.html

Ill  check it out and be back. Thanks.
0
 

Author Comment

by:Wanting2LearnMan
ID: 24250648
Hi, I have got my eVC++ flavour of your code working.  I'm still testing at the moment.  So far I have added several bits of data on the que and at present I'm just testing accessing my database from the threads and writing test data into the database.

my thread function is like:
DWORD WINAPI CMyClass::ThreadProc(LPVOID lpObj)
{
      CMyClass *pobjDlg = (CMyClass*)lpObj;
     
      //your coide here blah blah

      if(!s.empty())
      {
           pobjDlg->dbMyDatBase.OpenFile();//open my database
           pobjDlg->dbMyDatBase.m_sData1 = _T("blah");
           pobjDlg->dbMyDatBase.m_sData2 = _T("blahblah");
           pobjDlg->dbMyDatBase.SaveData();//save data
           pobjDlg->dbMyDatBase.CloseFile();//close database
     }
}

The database im using is codebase.  I am getting an error saying that a duplicate instance of the database cannot be opened.  I am thinking that the threads in the pool are trying to access the DB whenone thread already has it opened.  So do I have to add some sort of thread sync for access to this as well?

Any help or explanation on this on this is much appreciated.

Thanks
0
 
LVL 40

Expert Comment

by:evilrix
ID: 24250693
>> So do I have to add some sort of thread sync for access to this as well?
If the DB can only service one connection at a time then you'll need to sync access.

Have you tried opening a handle to the database in the main thread and passing that into your threads?

This is really an implementation detail of your database and not something I can really advise on.
0
 

Author Comment

by:Wanting2LearnMan
ID: 24251618
>>Have you tried opening a handle to the database in the main thread and passing that into your threads?
No, I'll try and investigate that.  Thanks.

>>If the DB can only service one connection at a time then you'll need to sync access.
Yes the db can only service one connection at a time.  Is this just as simple as setting an event whenever the database is free so another thread can access it.  If the event is not set then the threads sit about and wait like in your example?

Thanks
0
 
LVL 40

Expert Comment

by:evilrix
ID: 24252580
>> Is this just as simple as setting an event whenever the database is free so another thread can access it.
Use a mutex to provide mutual exclusion if you need to but you might find although it can only service one connection is can handle concurrent queries. If not it does somewhat mitigate the need for multiple threads in your thread pool... you might as well just have one thread servicing the queue.

>> If the event is not set then the threads sit about and wait like in your example?
The idea of the event in my example is to make them sleep until needed otherwise they'll be in a "busy- wait" chomping up CPU time.
0
 

Author Comment

by:Wanting2LearnMan
ID: 24256135
I understand your code to be doing the following: (Please correct me if I'm wrong)
1.  Several threads are created which wait for a job or a signal to quit
2.  Data is added to the queue.  If any of the worker threads are accessing the que (to process data) then this main thread waits until td.mtx is set.
3.  If the main thread is adding data to the que then the worker threads wait until the que is released
ANd so on..

NOW:
I dont think I told you my initial overall problem.
When a user taps a button I want the data stored in a database AND sent via wifi to a central server.  I want to be able to check the database to ensure each piece of data was sent (set a flag to indicate it was sent).  For performance reasons I do not want the Ui to slow down or freeze any.

So,I have a main UI thread (main program) which puts data into the que.
I will also have a thread (database thread)which is accessing the queue of data and writes that data to the database.
I will also have a thread which accesses the database and sends data via wifi (wifi thread).

I think I will have the following problem:

UI thread wants to add data to que but it has to wait on database thread which is waiting on the wifi thread which may be taking a long time to send the data.  This would result in the UI freezing.

Am I over complicating this or can you see a simple way to solve my problem?

Thanks again for all your help.
0
 

Author Comment

by:Wanting2LearnMan
ID: 24256378
Maybe I could go back to the original idea you had of starting up a new thread each time a button is pressed.  That way this new thread can go wait for database access as long as it needs to and the UI is free to work away.
0
 
LVL 40

Expert Comment

by:evilrix
ID: 24257996
>> Am I over complicating this or can you see a simple way to solve my problem?
May7be, why do you need separate threads for database and wifi? Why can't the same thread commit to the DB and then send over wifi? In this context a thread pool with multiple threads makes sense. The DB is only locked during the time taken to access it the Wifi part is done immediately afterwards after releasing the DB. IT doesn't matter if this thread takes a while because you have other threads servicing the UI.

>> Maybe I could go back to the original idea you had of starting up a new thread each time a button is pressed
I think a threadpool is still your best solution... managing a queue. The nice thing about this method is all yuser actions will be processed int he order they happen, this may or may not be important of course.

>> That way this new thread can go wait for database access as long as it needs to and the UI is free to work away.
NO different for a thread queue. Of course you only have a finite number of threads in a pool, whereas spawning on demand means you can spawn as many as you like but in reality available resources means there are only so many threads you can spawn and so you'll have to code to protect against spawning too many. If your UI has spawn the max threads then your UI will sit waiting until a new thread can be spawned, with the queue and threadpool there is no possibility of this happening.
0
 

Author Comment

by:Wanting2LearnMan
ID: 24258855
I'm getting somewhere with this now. I appreciate all your help.
In your example above, you add all the data to que in a loop at the beginning and then set an event telling threads to terminate when there are no more jobs in que.

I want to add data to the que when the user presses a button, like so:
ss= somedata
WaitForSingleOnject(td.mtx, INFINITE);
td.string_queue.push_back(ss);
ReleaseMutex(td.mtx);
SetEvent(td.evts[1]);

so there will be times when there is no data in the que as it has all peen processed, but the threads have all terminated.  
Do I:
not kill the threads when the que is empty?
OR
 start up a new thread pool if it is empty?
0
 
LVL 40

Expert Comment

by:evilrix
ID: 24258883
>> o there will be times when there is no data in the que as it has all peen processed, but the threads have all terminated.  

They will only terminate if you set the quit event (event 0)

>> not kill the threads when the que is empty?
No, they will just wait on the process event (event 1), which is a signal to tell them something is ready to be processed... since it's an automatic resetting event it'll just allow only 1 thread through to process what's been added. Everytime you add to the queue you trip this event to release a thread.

>> start up a new thread pool if it is empty?
A thread pool should live for the life of the program... the point of the pool (amongst other reasons stated above) is to avoid the cost of spinning up a new thread.

// Thread data
struct thread_data
{
      string_queue_t string_queue;
      HANDLE mtx;
      HANDLE evts[2]; // 0 Quit, 1 Process
};
0
 

Author Comment

by:Wanting2LearnMan
ID: 24258912
>>A thread pool should live for the life of the program
OK got it.

>>They will only terminate if you set the quit event (event 0)
So you just did this as part of your example, right?
0
 
LVL 40

Expert Comment

by:evilrix
ID: 24258923
>> So you just did this as part of your example, right?
You got it... but obviously you need to do the same when your application shuts down :)
0
 
LVL 49

Expert Comment

by:DanRollins
ID: 24287820
There is a simpler programming model that I have used successfully that avoids the complications of multi-threading altogether.  
Each time the user clicks the button, it adds a string to the CStringArray.  Then I have a Window Timer that runs, say every 100ms that checks to see if there is anything in the array, if so, it takes action (update database, delete item from array).
This works if the database update takes less time than the expected interval of button clicks.  Of course most database updates will take less than 1ms, so unless the user clicks the button 1000 times per second, this works fine.  If the database update takes longer than a few ms, then it is probably because you are opening and closing the connection too often.  Open it once and keep it open.
An enhancement would be for the Timer to process multiple items... For instance, it could keep processing until a PeekMessage indicated that a new button click was coming in.
Because there is only one thread, there is no danger of "race condition" no need for mutexes, etc.  The Timer proc may get behind a bit (if you are working with a slow database), but it will eventually catch up.  
Simpler is better.  If you need to see code for this, I'll be glad to provide it.
-- Dan
0
 

Author Comment

by:Wanting2LearnMan
ID: 24289747
Hi Dan, thanks for your input on this topic, all help is appreciated.  I like the simple solution you are talking about but how would it handle the 'sending' to the server.  If I have only one thread will this not slow down the operation of the UI?  Lets say  the sending to the server took a few seconds then the UI would have to wait on this???

Thanks.
0
 
LVL 40

Expert Comment

by:evilrix
ID: 24290437
>> If I have only one thread will this not slow down the operation of the UI?
Unless the timer proc runs via the main thread (which I'm not convinced it does) you still have 2 threads so you still need mutual exclusion protection. If it does run in the main thread then if there are a lot of DB requests this may still block the UI for the duration.
0
 
LVL 49

Expert Comment

by:DanRollins
ID: 24291392
Window Timers work by sending a WM_TIMER message to your window.  Your program processes these and other messages (WM_COMMAND, WM_RBUTTONDOWN. WM_PAINT, etc.) sequentially.  In a normal message-pump scenario, no new message is processed until the previous message handler returns.  Consider that the time between two clicks or the time while a user reads a label is an eternity; your CPU meter rarely climbs above 10% so there is plenty of time to get lots of stuff done while the user blinks his eyes.
>> how would it handle the 'sending' to the server.
This could be problematic if this operation can get stalled.  For instance, if the server stops accepting data, you could end up with your main thread waiting for a timeout.  Sometimes, you can use asynchronous I/O (depending on your 'send' technique).  
Or you could spin off a thread for just this task.  All it does is read from the database and send some resulting data somewhere.  Database access is inherently thread-safe.  If you so, say:
   SELECT COUNT(*) FROM MyTable WHERE notSentYet=1
then (if non-zero)...
   SELECT * FROM MyTable WHERE notSentYet=1 ORDER BY tTimestamp
... the resulting snapshot recordset  will contain only those records that are ready to go.   The problem with multi-threading that uses in-memory queues is the chance that there might be a task-switch when the data has been partially updated (e.g., MyRec.Name has been recorded, but not MyRec.Address).  That's not a problem with database access.
0

Featured Post

Hire Technology Freelancers with Gigs

Work with freelancers specializing in everything from database administration to programming, who have proven themselves as experts in their field. Hire the best, collaborate easily, pay securely, and get projects done right.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

In this post we will learn different types of Android Layout and some basics of an Android App.
Article by: evilrix
Looking for a way to avoid searching through large data sets for data that doesn't exist? A Bloom Filter might be what you need. This data structure is a probabilistic filter that allows you to avoid unnecessary searches when you know the data defin…
The viewer will learn how to pass data into a function in C++. This is one step further in using functions. Instead of only printing text onto the console, the function will be able to perform calculations with argumentents given by the user.
The viewer will be introduced to the technique of using vectors in C++. The video will cover how to define a vector, store values in the vector and retrieve data from the values stored in the vector.
Suggested Courses
Course of the Month18 days, 14 hours left to enroll

834 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