Multithreading design

[cyberizen]

Hi

I have come up with a Singleton class that manages a pool of database connections. Basically the pool is a queue in which I have stored some connection objects. When ever a client calls any of the database class methods a connection object is popped from the queue and when done the connection is pushed on the queue. If the there are no more connections then that thread has to wait for signal when the pool has a connection then a signal is broadcasted to the wait thread. And now some code

if(dbPool.size() != 0)
{
               
     pthread_mutex_lock(&criticalSection);
     DNativeDb activeDbObj = dbPool.pop();
     pthread_mutex_lock(&criticalSection);
     ...
}

First Question: Is it ok to put pop() under the a mutex_lock

elsewhere in code

pthread_mutex_lock(&criticalSection);
dbPool.push(activeObj);
if(dbPool.size() == 1)
{
     pthread_cond_broadcast(&cond);
}
thread_mutex_lock(&criticalSection);

Second: When my pool has 1 connection object left then I need to broadcast incase some thread that might be waiting can then acquire the object. Is this the right way to achieve pool based connecting handling. Please guide and suggest if I am going wrong anywhere.

Thanks
Ahmed Bhaila

[RahulKateWin32]

Hi,
From ur question u want to check whatever u designed for Multithreading project is it correct or not.If u r giving a detailed design abt how many thread in ur project how it synchronize then and only draw the conclusion abt ur design.

Rahul U Kate

[cyberizen]

Hi

I have come up with this code that I posted. I want to know is this the right approach. My object is to create a wrapper database class that manages connections and handles synchronization when pushing popping using the queue. Is this the right way to do this?

[Arjay]

I'm a little out of my element here since my multithreading experience is on Windows, not Unix, but I believe the concepts are similar (I'm sure someone will correct me if I'm wrong).

When you have container that stores objects, you need to lock the container (queue in this case) when you add and remove items. You also don't want to remove an item while it's in the middle of an operation.

When looking at your code, I would be careful of the placement of the synchronization sections and strive for locking for the minimum amount of time (always a good rule of thumb).

With this in mind, I would suggest creating some simple queue access methods that allow you to push, pop, and here's the kicker, retrieve the first item in the queue (without popping it).

For some code, check out the LogSend example (LogSend.cpp) in the article Win32 Thread Synchronization, Part 2: Helper Classes. (sorry, this code isn't a UNIX example, but you may find the techniques helpful)

This example uses this technique to process fictitious log entries from within a queue - one thread adds log items to a queue while another thread takes the items off the queue and sends them to another process via a memory mapped file. But you don't care about the interprocess communication via mmf's, just look at the queuing code as it provides the queue synchronization with the following methods:

 LPLOGITEM GetNextLogItemFromQueue() 
void PopLogItemFromQueue()
void InsertLogItem( LPLOGITEM pLI )

Using the above methods, the following code sits in a secondary thread and processes the queue items:

 
// Retrieve the next log item from the queue
while(NULL != (pLI = pLogSender->GetNextLogItemFromQueue())) 
{
// Send the log item off to the other process
if(FAILED(hr = pLogSender->SendLogItem(pLI)))
{
	return hr;
}
 
// Pop the item from the Queue
pLogSender->PopLogItemFromQueue( );
 
// Check if we should exit
if( WAIT_OBJECT_0 
	== WaitForSingleObject( pLogSender->GetShutdownEvent( ), 0 ) )
{
	return S_FALSE;
}
}

The whole point of the article is to use the idea of RAII (Resource Acquisition Is Initialization) to help with thread synchronization chores. The article introduces some helper classes that help with locking and unlocking of shared resources. These classes make the thread synchronization simple. For example, here's the code for popping the item off the queue:

 void PopLogItemFromQueue() 
{
  CAutoLockT< CLogItemQueue > lock(&m_LogItemQueue);
  if(!m_LogItemQueue.empty())
  {
	m_LogItemQueue.pop();
  }
}

I'm not trying to push the autolock class on you, (these classes are Win32 specific anyways), but using the RAII concept can really make life easy when dealing with thread synchronization. At first glance, it only saves you a line of code (ie. you don't need to explicitly make a call to unlock), but that's not the real benefit. The real benefit is that by when the AutoLock instance goes out of scope, the resource gets unlocked - even if the code throws an exception in the middle of an operation unlock will get called automatically as the stack unwinds.

Arjay

[righteous]

Why would you want a thread to wait for a DB connection? If there are no more connections left why not just return a 0 and get the thread to look after trying to get a connection again after a short time. Also why not create a temporary connection and give it to a thread if all the cached connections are in use. You can then clean up this temp connection.

If you unblock all threads they will all rush in and compete for the mutex wouldn't they?

[Barriebyrnes]

Its good information about design. Design code and program are really important for any site and project program.

[Arjay]

Why would you want a thread to wait for a DB connection? If there are no more connections left why not just return a 0 and get the thread to look after trying to get a connection again after a short time. Also why not create a temporary connection and give it to a thread if all the cached connections are in use. You can then clean up this temp connection.

If you unblock all threads they will all rush in and compete for the mutex wouldn't they?

Since this reply was posted in August, it might be a bit late for a reply. But in general, what else is the thread going to do while waiting for the resource to free up? Are you putting the thread to sleep while waiting until you retry? If so, then it's no better than letting the OS scheduler schedule your thread.