Archived

This forum has been archived. Please start a new discussion on GitHub.

Question about demo\IceUtil\workqueue\WorkQueue.cpp

Yunqiao Yin
Yunqiao Yin
in Bug Reports
The codes in method add(...) is here: 
add(const string& item)
    {
	IceUtil::Monitor<IceUtil::Mutex>::Lock lock(_monitor);
	if(_queue.empty())
	{
	    _monitor.notify();
	}
	_queue.push_back(item);
    }
I think above codes should be modified into this: 
add(const string& item)
    {
	IceUtil::Monitor<IceUtil::Mutex>::Lock lock(_monitor);
	_queue.push_back(item);
	if(_queue.size()==1)
	{
	    _monitor.notify();
	}
    }
I think the original code will be result into contentention between read threads and writer threads.

Comments

  • matthew
    matthew NL, Canada
    If you do that the events will be thrown out unless the queue is empty. That's not much of a work queue :) Could you detail more precisely what contention problem you are trying to address?
  • Yunqiao Yin
    Yunqiao Yin
    matthew wrote:
    If you do that the events will be thrown out unless the queue is empty. That's not much of a work queue :) Could you detail more precisely what contention problem you are trying to address?
    Sorry,I wrote wrong codes above;) I mean this 
        add(const string& item)
    {
	IceUtil::Monitor<IceUtil::Mutex>::Lock lock(_monitor);
	_queue.push_back(item);
	if(_queue.size()==1)
	{
	    _monitor.notify();
	}
    }
    Otherwise,contentation maybe 
        //writer thread
    add(const string& item)
    {
	IceUtil::Monitor<IceUtil::Mutex>::Lock lock(_monitor);
	if(_queue.empty())  
	{
	    _monitor.notify();   /// no. 2
	}
	_queue.push_back(item); /// no.6
    }
    and the reader thread will be 
     string    nextItem()
    {
        IceUtil::Monitor<IceUtil::Mutex>::Lock lock(_monitor);  //No.4
        while(_queue.empty())
        {
            _monitor.wait();  //No. 1  & No.5
        }

        string item = _queue.front(); // No.3
        _queue.pop_front();
        return item;
    }

    If one reader and one writer threads runs following the given sequence No.1 - No.6 , the reader will wait forever,and the writer will never gives notify signal to the reader.
  • Yunqiao Yin
    Yunqiao Yin
    Oh,maybe I misunderstand the monitor's semantic. Page 711 of Ice-3.1.1.pdf
    notify
    This function wakes up a single thread that is currently suspended in a call to
    wait. If no thread is suspended in a call to wait at the time notify is
    called, the notification is lost (that is, calls to notify are not remembered if
    there is no thread to be woken up).
    Note that notifying does not run another thread immediately. Another thread
    gets to run only once the notifying thread either calls wait or unlocks the
    monitor (Mesa semantics).
    If monitor is in keeping with the Mesa semantics,the above contention will never occur,but I had really met the condition which the reader waited forever while the work queue's size kept increasing. :confused:
  • matthew
    matthew NL, Canada
    Since the monitor does implement the mesa semantics this should not occur. Only one thread can hold the monitor lock at a time.

    In this case it is either the reader or a writer. If the reader holds the lock (calling nextItem) then if the queue is empty it waits for a notification (thus releasing the lock and waiting for the monitor to be notified), or it dequeues the head item. If the writer holds the lock (calling add) then if the queue is empty it wakes any sleeping readers and then enqueues the item. Note that the reader does not wake until add returns.

    Did you meet the error condition you describe with this demo? Or some other application?

Categories