Semaphore potentially may underflow and become negative

Bug #617021 reported by Gennady Proskurin on 2010-08-12
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
DC++
Medium
Unassigned
LinuxDC++
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;
                        }

Related branches

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
Steven Sheehy (steven-sheehy) wrote :

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

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.

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
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  Edit
Everyone can see this information.

Other bug subscribers