Race condition in NotificationQueue?

[oswaldm]

Hello,


I'm currently evaluating Poco for use in a project and wrote some small demo programs. One of them was the use of the NofiticationQueue as communication between two threads. The program simply spawns a thread and sends some notifications to the other thread which simply logs them to cout.

I did a run of the application with the valgrind tool helgrind (the data race detector) and it reported the following:

Code: Select all


==5891== Possible data race during read of size 4 at 0x440DB28
==5891==    at 0x412A654: Poco::NotificationQueue::waitDequeueNotification() (NotificationQueue.cpp:113)
==5891==    by 0x8049E78: NotificationThread::run() (NotificationThread.cpp:67)
==5891==    by 0x415BD9A: Poco::ThreadImpl::entry(void*) (Thread_POSIX.cpp:186)
==5891==    by 0x401D8C6: mythread_wrapper (hg_intercepts.c:193)
==5891==    by 0x43F4CF6: start_thread (in /lib/tls/libpthread.so.0)
==5891==    by 0x438F2ED: clone (in /lib/tls/libc.so.6)
==5891==    by 0x500CBAF: ???
==5891==   Old state: shared-modified by threads #1, #2
==5891==   New state: shared-modified by threads #1, #2
==5891==   Reason:    this thread, #2, holds no consistent locks
==5891==   Last consistently used lock for 0x440DB28 was first observed
==5891==    at 0x401DBA2: pthread_mutex_init (hg_intercepts.c:346)
==5891==    by 0x439ACB2: pthread_mutex_init (in /lib/tls/libc.so.6)
==5891==    by 0x412656F: Poco::MutexImpl::MutexImpl(bool) (Mutex_POSIX.cpp:83)
==5891==    by 0x41269A7: Poco::FastMutexImpl::FastMutexImpl() (Mutex_POSIX.cpp:141)
==5891==    by 0x4126AFF: Poco::FastMutex::FastMutex() (Mutex.cpp:61)
==5891==    by 0x412A10D: Poco::NotificationQueue::NotificationQueue() (NotificationQueue.cpp:47)
==5891==    by 0x8049B92: NotificationThread::NotificationThread() (NotificationThread.cpp:39)
==5891==    by 0x80494B9: main (main.cpp:15)


I looked up waitDequeueNotification:

Code: Select all


Notification* NotificationQueue::waitDequeueNotification()
{
   Notification* pNf = 0;
   WaitInfo*     pWI = 0;
   {
      FastMutex::ScopedLock lock(_mutex);
      pNf = dequeueOne();
      if (pNf) return pNf;
      pWI = new WaitInfo;
      pWI->pNf = 0;
      _waitQueue.push_back(pWI);
   }
   pWI->nfAvailable.wait();
   pNf = pWI->pNf;                  <--- valgrind complains about this line
   delete pWI;
   return pNf;
}


I have not looked too deep into the code, so I can't tell if this is a false-positive or if this is a real problem.

Can somebody clarify this?



lg,
Michael

[alex]

^> I have not looked too deep into the code, so I can't tell if this is a false-positive or if this is a real problem.
>
> Can somebody clarify this?^

Since both pNf and pWI are local variables, every thread will have its own copy of those pointers, so I don't see how race condition could happen.

I suspect the reason the tool is complaining is due to it's non-understanding of the blocking nature of the wait() call.

Alex

[guenter]

That's definitely a false-positive.
The access to the wait queue is protected by a Mutex. And by the time pWI->nfAvailable.wait() returns, waitDequeueNotification() has exclusive access to the WaitInfo object (see also enqueueNotification(), which signals the _nfAvailable event).

Furthermore, the code in question is one of the most used parts of POCO - NotificationQueue is used by the HTTP server, which has been in heavy use for more than 3 years now.

Btw, please let us know about the result of your evaluation. If you don't want to do it in a public forum, you can also reach us at poco at appinf.com.

Thanks,

Günter

[oswaldm]

> Since both pNf and pWI are local variables, every thread will have its own copy of those pointers, so I don't see how race condition could happen.
>
> I suspect the reason the tool is complaining is due to it's non-understanding of the blocking nature of the wait() call.


Yes, that's possible. As I understand it, it creates internally a graph of variables which are accessed from multiple threads and then looks, if they are protected by a mutex.

Well, I already had some false-positives with helgrind in some other code, so I just wanted to be sure.

Thanks a lot guys!


lg,
Michael