From: Johannes Sixt <j6t@kdbg.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Zoltán Füzesi" <zfuzesi@eaglet.hu>,
git@vger.kernel.org,
"msysGit Mailinglist" <msysgit@googlegroups.com>,
"Dmitry Potapov" <dpotapov@gmail.com>,
"Andrzej K. Haczewski" <ahaczewski@gmail.com>,
"Erik Faye-Lund" <kusmabite@googlemail.com>
Subject: [PATCH] Implement pthread_cond_broadcast on Windows
Date: Sat, 30 Jan 2010 00:54:05 +0100 [thread overview]
Message-ID: <201001300054.05610.j6t@kdbg.org> (raw)
In-Reply-To: <201001292316.03858.j6t@kdbg.org>
See http://www.cse.wustl.edu/~schmidt/win32-cv-1.html, section "The
SignalObjectAndWait solution". But note that this implementation does not
use SignalObjectAndWait (which is needed to achieve fairness, but we do
not need fairness).
Note that our implementations of pthread_cond_broadcast and
pthread_cond_signal require that they are invoked with the mutex held that
is used in the pthread_cond_wait calls.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Junio,
please queue this patch for 1.7.0-rc1 even though it has not undergone
a lot of review - the result is better than a git that does not even
build on Windows.
I had another look at this complete implementation of p_c_b and am now
convinced that it is correct, but an extra set of eye-balls is always
appreciated. I've Cc'd people who had shown interest in pthreads on
Windows in the past.
-- Hannes
compat/win32/pthread.c | 100 ++++++++++++++++++++++++++++++++++++++++++-----
compat/win32/pthread.h | 9 ++--
2 files changed, 94 insertions(+), 15 deletions(-)
diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 631c0a4..e592084 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -52,24 +52,38 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
int pthread_cond_init(pthread_cond_t *cond, const void *unused)
{
cond->waiters = 0;
+ cond->was_broadcast = 0;
+ InitializeCriticalSection(&cond->waiters_lock);
cond->sema = CreateSemaphore(NULL, 0, LONG_MAX, NULL);
if (!cond->sema)
die("CreateSemaphore() failed");
+
+ cond->continue_broadcast = CreateEvent(NULL, /* security */
+ FALSE, /* auto-reset */
+ FALSE, /* not signaled */
+ NULL); /* name */
+ if (!cond->continue_broadcast)
+ die("CreateEvent() failed");
+
return 0;
}
int pthread_cond_destroy(pthread_cond_t *cond)
{
CloseHandle(cond->sema);
- cond->sema = NULL;
-
+ CloseHandle(cond->continue_broadcast);
+ DeleteCriticalSection(&cond->waiters_lock);
return 0;
}
int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)
{
- InterlockedIncrement(&cond->waiters);
+ int last_waiter;
+
+ EnterCriticalSection(&cond->waiters_lock);
+ cond->waiters++;
+ LeaveCriticalSection(&cond->waiters_lock);
/*
* Unlock external mutex and wait for signal.
@@ -82,22 +96,52 @@ int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)
/* let's wait - ignore return value */
WaitForSingleObject(cond->sema, INFINITE);
- /* we're done waiting, so make sure we decrease waiters count */
- InterlockedDecrement(&cond->waiters);
-
+ /*
+ * Decrease waiters count. If we are the last waiter, then we must
+ * notify the broadcasting thread that it can continue.
+ * But if we continued due to cond_signal, we do not have to do that
+ * because the signaling thread knows that only one waiter continued.
+ */
+ EnterCriticalSection(&cond->waiters_lock);
+ cond->waiters--;
+ last_waiter = cond->was_broadcast && cond->waiters == 0;
+ LeaveCriticalSection(&cond->waiters_lock);
+
+ if (last_waiter) {
+ /*
+ * cond_broadcast was issued while mutex was held. This means
+ * that all other waiters have continued, but are contending
+ * for the mutex at the end of this function because the
+ * broadcasting thread did not leave cond_broadcast, yet.
+ * (This is so that it can be sure that each waiter has
+ * consumed exactly one slice of the semaphor.)
+ * The last waiter must tell the broadcasting thread that it
+ * can go on.
+ */
+ SetEvent(cond->continue_broadcast);
+ /*
+ * Now we go on to contend with all other waiters for
+ * the mutex. Auf in den Kampf!
+ */
+ }
/* lock external mutex again */
EnterCriticalSection(mutex);
return 0;
}
+/*
+ * IMPORTANT: This implementation requires that pthread_cond_signal
+ * is called while the mutex is held that is used in the corresponding
+ * pthread_cond_wait calls!
+ */
int pthread_cond_signal(pthread_cond_t *cond)
{
- /*
- * Access to waiters count is atomic; see "Interlocked Variable Access"
- * http://msdn.microsoft.com/en-us/library/ms684122(VS.85).aspx
- */
- int have_waiters = cond->waiters > 0;
+ int have_waiters;
+
+ EnterCriticalSection(&cond->waiters_lock);
+ have_waiters = cond->waiters > 0;
+ LeaveCriticalSection(&cond->waiters_lock);
/*
* Signal only when there are waiters
@@ -108,3 +152,37 @@ int pthread_cond_signal(pthread_cond_t *cond)
else
return 0;
}
+
+/*
+ * DOUBLY IMPORTANT: This implementation requires that pthread_cond_broadcast
+ * is called while the mutex is held that is used in the corresponding
+ * pthread_cond_wait calls!
+ */
+int pthread_cond_broadcast(pthread_cond_t *cond)
+{
+ EnterCriticalSection(&cond->waiters_lock);
+
+ if ((cond->was_broadcast = cond->waiters > 0)) {
+ /* wake up all waiters */
+ ReleaseSemaphore(cond->sema, cond->waiters, NULL);
+ LeaveCriticalSection(&cond->waiters_lock);
+ /*
+ * At this point all waiters continue. Each one takes its
+ * slice of the semaphor. Now it's our turn to wait: Since
+ * the external mutex is held, no thread can leave cond_wait,
+ * yet. For this reason, we can be sure that no thread gets
+ * a chance to eat *more* than one slice. OTOH, it means
+ * that the last waiter must send us a wake-up.
+ */
+ WaitForSingleObject(cond->continue_broadcast, INFINITE);
+ /*
+ * Since the external mutex is held, no thread can enter
+ * cond_wait, and, hence, it is safe to reset this flag
+ * without cond->waiters_lock held.
+ */
+ cond->was_broadcast = 0;
+ } else {
+ LeaveCriticalSection(&cond->waiters_lock);
+ }
+ return 0;
+}
diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
index b8e1bcb..c72f100 100644
--- a/compat/win32/pthread.h
+++ b/compat/win32/pthread.h
@@ -32,17 +32,18 @@
* See also: http://www.cse.wustl.edu/~schmidt/win32-cv-1.html
*/
typedef struct {
- volatile LONG waiters;
+ LONG waiters;
+ int was_broadcast;
+ CRITICAL_SECTION waiters_lock;
HANDLE sema;
+ HANDLE continue_broadcast;
} pthread_cond_t;
extern int pthread_cond_init(pthread_cond_t *cond, const void *unused);
-
extern int pthread_cond_destroy(pthread_cond_t *cond);
-
extern int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex);
-
extern int pthread_cond_signal(pthread_cond_t *cond);
+extern int pthread_cond_broadcast(pthread_cond_t *cond);
/*
* Simple thread creation implementation using pthread API
--
1.6.6.264.ga6155
next prev parent reply other threads:[~2010-01-29 23:55 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-29 10:27 threaded-grep cause msys build failure Zoltán Füzesi
2010-01-29 12:03 ` Johannes Sixt
2010-01-29 19:26 ` [RFC/PATCH] MSVC: Windows-native implementation of pthread_cond_broadcast Zoltán Füzesi
2010-01-29 20:02 ` Johannes Sixt
2010-01-29 22:16 ` [PATCH] Windows: a minimal pthread_cond_broadcast Johannes Sixt
2010-01-29 23:54 ` Johannes Sixt [this message]
2010-01-30 2:28 ` [PATCH] Implement pthread_cond_broadcast on Windows Johannes Schindelin
2010-01-30 9:30 ` Johannes Sixt
2010-01-30 10:50 ` Johannes Schindelin
2010-01-29 20:13 ` [RFC/PATCH] MSVC: Windows-native implementation of pthread_cond_broadcast Johannes Schindelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201001300054.05610.j6t@kdbg.org \
--to=j6t@kdbg.org \
--cc=ahaczewski@gmail.com \
--cc=dpotapov@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=kusmabite@googlemail.com \
--cc=msysgit@googlegroups.com \
--cc=zfuzesi@eaglet.hu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).