[Bordeaux-threads-devel] [Updated]Patch for ABCL against BORDEAUX-THREADS HEAD
Please consider the [following patch for ABCL against BORDEAUX-THREAD HEAD][1]. The patch provides implementation of both BT locking semantics (recursively head and not) by use of [java.util.concurrent.locks ReentrantLock][2] as well as a (hopefully) correct implementation of POSIX-style condition variable, and allows non-blocking lock acquisition. [1]: http://slack.net/~evenson/abcl/hunchentoot/bordeaux-threads-abcl-20110215a.d... [2]; http://download.oracle.com/javase/6/docs/api/java/util/concurrent/locks/Reen... -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
On 2/15/11 10:20 PM, Mark Evenson wrote:
[1]: http://slack.net/~evenson/abcl/hunchentoot/bordeaux-threads-abcl-20110215a.d...
Err, [use 'detroit.slack.net' in the above][1]. [1]: http://detroit.slack.net/~evenson/abcl/bordeaux-threads-abcl-20110216a.diff Attached as well… -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
After critique from Stellian, I have [successfully brought my patches in alignment with BORDEAUX-THREADS GIT head][1]. Please review this for inclusion. [1]: http://slack.net/~evenson/abcl/bordeaux-threads-abcl-20110301a.diff -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
On Tue, 01 Mar 2011 16:18:35 +0100, Mark Evenson said:
After critique from Stellian, I have [successfully brought my patches in alignment with BORDEAUX-THREADS GIT head][1].
Please review this for inclusion.
[1]: http://slack.net/~evenson/abcl/bordeaux-threads-abcl-20110301a.diff
I think that the implementation of condition-wait has some major problems: 1) If no other threads are trying to claim the lock, then condition-wait will return immediately rather than waiting. 2) If two threads are waiting in condition-wait and two other threads call condition-notify, then it is possible that only one thread will return from condition-wait because the call to acquire-lock in one of them might return nil causing it to wait again. 3) If condition-notify is called by one thread when a waiting thread is just about to enter the threads:synchronized-on form (but before it gets synchronized), then the notify will be lost. This happens because the underlying threading primitives have no "memory" of calls to notify when nothing it waiting (which is also the expected semantics for POSIX condition variables BTW). Also, I think condition-notify should be using threads:object-notify rather than threads:object-notify-all. -- Martin Simmons LispWorks Ltd http://www.lispworks.com/
On 3/2/11 8:50 PM, Martin Simmons wrote:
I think that the implementation of condition-wait has some major problems:
Thanks very much for the comments. I have an [updated patch][1] that attempts to address these issues for further review for inclusion in BORDEAUX-THREADS. [1]: http://detroit.slack.net/~evenson/abcl/bordeaux-threads-abcl-20110318a.diff
1) If no other threads are trying to claim the lock, then condition-wait will return immediately rather than waiting.
I couldn't reproduce this behavior. In ABCL, the THREADS:OBJECT-WAIT implementation is mainly a simple wrapping of the underlying [java.lang.Object.wait()][2] which clearly should wait. [2]: http://download.oracle.com/javase/6/docs/api/java/lang/Object.html#wait%28%2...
2) If two threads are waiting in condition-wait and two other threads call condition-notify, then it is possible that only one thread will return from condition-wait because the call to acquire-lock in one of them might return nil causing it to wait again.
The DO looping construct has been replaced with a simpler subsequent BT:ACQUIRE-LOCK that should solve this bug.
3) If condition-notify is called by one thread when a waiting thread is just about to enter the threads:synchronized-on form (but before it gets synchronized), then the notify will be lost. This happens because the underlying threading primitives have no "memory" of calls to notify when nothing it waiting (which is also the expected semantics for POSIX condition variables BTW).
THREADS:SYNCHRONIZED-ON is now the first form executed by BT:CONDITION-WAIT so the window here is considerably narrowed.
Also, I think condition-notify should be using threads:object-notify rather than threads:object-notify-all.
We now use THREADS:OBJECT-NOTIFY. -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
On Fri, 18 Mar 2011 08:50:50 +0100, Mark Evenson said:
On 3/2/11 8:50 PM, Martin Simmons wrote:
I think that the implementation of condition-wait has some major problems:
Thanks very much for the comments.
I have an [updated patch][1] that attempts to address these issues for further review for inclusion in BORDEAUX-THREADS.
[1]: http://detroit.slack.net/~evenson/abcl/bordeaux-threads-abcl-20110318a.diff
That looks good to me.
3) If condition-notify is called by one thread when a waiting thread is just about to enter the threads:synchronized-on form (but before it gets synchronized), then the notify will be lost. This happens because the underlying threading primitives have no "memory" of calls to notify when nothing it waiting (which is also the expected semantics for POSIX condition variables BTW).
THREADS:SYNCHRONIZED-ON is now the first form executed by BT:CONDITION-WAIT so the window here is considerably narrowed.
Yes, that fixes it for all correct uses (those that call condition-notify while holding the lock). -- Martin Simmons LispWorks Ltd http://www.lispworks.com/
On Fri, 2011-03-18 at 08:50 +0100, Mark Evenson wrote:
On 3/2/11 8:50 PM, Martin Simmons wrote:
I think that the implementation of condition-wait has some major problems:
Thanks very much for the comments.
I have an [updated patch][1] that attempts to address these issues for further review for inclusion in BORDEAUX-THREADS.
I've applied the patch, with one modification: release-recursive-lock calls "unlock" only once, because we require that acquisition and release be balanced -- Stelian Ionescu a.k.a. fe[nl]ix Quidquid latine dictum sit, altum videtur. http://common-lisp.net/project/iolib
participants (3)
-
Mark Evenson -
Martin Simmons -
Stelian Ionescu