Semaphore potentially may underflow and become negative

Bug #617021 reported by Gennady Proskurin
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
DC++
Fix Released
Medium
Unassigned
LinuxDC++
Confirmed
Medium
Unassigned

Bug Description

As noted in POSIX about pthread_cond_wait, spurious wakeup may occur and predicate should be rechecked after wakeup.
So the patch.

diff --git a/client/Semaphore.h b/client/Semaphore.h
index f89148d..b0c59e5 100644
--- a/client/Semaphore.h
+++ b/client/Semaphore.h
@@ -61,7 +61,7 @@ public:

        bool wait() throw() {
                Lock l(cs);
- if(count == 0) {
+ while (count == 0) {
                        pthread_cond_wait(&cond, &cs.getMutex());
                }
                count--;
@@ -76,7 +76,10 @@ public:
                        millis+=timev.tv_usec/1000;
                        t.tv_sec = timev.tv_sec + (millis/1000);
                        t.tv_nsec = (millis%1000)*1000*1000;
- int ret = pthread_cond_timedwait(&cond, &cs.getMutex(), &t);
+ int ret;
+ do {
+ ret = pthread_cond_timedwait(&cond, &cs.getMutex(), &t);
+ } while (ret==0 && count==0);
                        if(ret != 0) {
                                return false;
                        }

Tags: core

Related branches

Revision history for this message
Razzloss (razzloss) wrote :

True. And this seems to affect the dev version also. I assume (from the fact that there's still client dir) this was from/for the 1.0.3?

--RZ

Changed in linuxdcpp:
importance: Undecided → Medium
milestone: none → 1.1.0
status: New → Confirmed
tags: added: core
Revision history for this message
Steven Sheehy (steven-sheehy) wrote :

Thanks for the patch, but in the future please attach it as a separate text file.

Revision history for this message
Gennady Proskurin (gpr) wrote :

This patch is for 1.0.3 version, which I use. I didn't notice newer version existence until now :)
Not sure if this patch solves some real bug, but I think it should be applied anyway.
Sorry for supplying patch in-place instead of separate file.

Revision history for this message
Steven Sheehy (steven-sheehy) wrote :

Arne committed to dcpp. Will make its way to ldcpp once we upgrade the core at some point in the future.

Changed in dcplusplus:
importance: Undecided → Medium
status: New → Fix Committed
Revision history for this message
poy (poy) wrote :

Fixed in DC++ 0.780.

Changed in dcplusplus:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.