Modify

Opened 16 years ago

Closed 12 years ago

#80 closed Bugs (Fixed)

Bosot.Thread.condition(win32).notify_all

Reported by: nobody Owned by: glassfordm
Milestone: Component: threads
Version: None Severity:
Keywords: Cc:

Description

Problem:this is a bug in the notify_all() methode in
        "Boost.Thread.Condition(win32)".

Why:the bug causes a deadlock in my machine.

Desc: ->suppose that i have two thread waiting for
      a condition, then the condition state will be
      ( m_block = 2, m_waiting = 0, m_gon = 0,
        the queue semaphore's count become = -2,
        and the mutex state unlocked).
      ->then suppose that there is another thread call
      the notify_one() methode of the shared condition
      ,thus the state of the condition become
      (m_block = 1, m_waiting = 1, m_gon = 0,
       the queue semaphore's count = -1,
       and the mutex state unlocked)
      -> then this thread or another reschudled before
         the waked up thread from the queue and 
         called the notify_all() methode, it will
         find that m_waiting != 0 , and m_block != 0
         and the state of the condition become after
         this call
        (m_block = 0, m_waiting = 2, m_gon = 0,
         the queue semaphore's count = -1(bug),
         and the muext state locked(deadlock)).
     **-> thus when we noted that there are  a bug in
          the queue semaphore's count, and a deadlock
          in the mutex state that was unlocked.
      
Bug Fix: here is the new version of the methode after
         correcting the bug.
void condition::notify_all()
{
    unsigned signals = 0;
    int res;

    res = WaitForSingleObject(reinterpret_cast<HANDLE>
             (m_mutex), INFINITE);
    assert(res == WAIT_OBJECT_0);

    if (m_waiting != 0) // the m_gate is already closed
    {
        if (m_blocked == 0)
        {
            res = ReleaseMutex(reinterpret_cast<HANDLE>
                              (m_mutex));
            assert(res);
            return;
        }

        m_waiting += (signals = m_blocked);
        m_blocked = 0;
    }
    else
    {
        res = WaitForSingleObject
         (reinterpret_cast<HANDLE>(m_gate), INFINITE);
        assert(res == WAIT_OBJECT_0);
        if (m_blocked > m_gone)
        {
            if (m_gone != 0)
            {
                m_blocked -= m_gone;
                m_gone = 0;
            }
            signals = m_waiting = m_blocked;
            m_blocked = 0;
        }
        else
        {
            res = ReleaseSemaphore
               (reinterpret_cast<HANDLE>(m_gate), 1,0);
            assert(res);
        }
    }

   res = ReleaseMutex(reinterpret_cast<HANDLE>
                             (m_mutex));
   assert(res);

   if (signals)
   {
      res = ReleaseSemaphore(reinterpret_cast<HANDLE>
            (m_queue), signals, 0);
      assert(res);
   }

Note : sorry for my bed english.

name: walead mohammed.
thanks.

Attachments (0)

Change History (1)

comment:1 Changed 12 years ago by Markus Schöpflin

Status: assignedclosed
Logged In: YES 
user_id=91733

From source code review I concluded that the current CVS
version contains exactly the code given above. Therefore I
consider this as fixed.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain glassfordm.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.