From: Tobias C. R. <tc...@fr...> - 2009-07-11 14:05:13
|
I tried to use the new thread interface in Clisp's SWANK backend, and I have come across a few minor issues, and one serious bug: * The lock-related functions are consistently called MUTEX-foo, but the macro to grab a mutex is called WITH-LOCK. * I don't like that (thread-name (make-thread #'(lambda ()))) returns #<UNBOUND>, as you cannot use (or (thread-name ...) "SOME-DEFAULT") this way. * I think the default print-object method specializing on threads should print whether the thread is currently running, or has been stopped. Now to a much more serious issue: * If I run the file below, clisp exits with "Aborted". I'm on Linux-x86/32, compiled clisp from CVS four hours ago. Have threads and weak-hashtables been tested? -T. PS. (defvar *thread-plist-table-lock* (mp:make-mutex :name "THREAD-PLIST-TABLE-LOCK")) (defvar *thread-plist-table* (make-hash-table :weak :key) "A hashtable mapping threads to a plist.") (defvar *thread-id-counter* 0) (defun thread-id (thread) (mp:with-lock (*thread-plist-table-lock*) (or (getf (gethash thread *thread-plist-table*) 'thread-id) (setf (getf (gethash thread *thread-plist-table*) 'thread-id) (incf *thread-id-counter*))))) (defvar *thread* (make-thread (lambda () (loop (sleep 1))))) (thread-id *thread*) (thread-interrupt *thread* :function t) (setq *thread* nil) (gc) |
From: Vladimir T. <vtz...@gm...> - 2009-07-11 21:16:15
|
On 7/11/09, Tobias C. Rittweiler <tc...@fr...> wrote: > > I tried to use the new thread interface in Clisp's SWANK backend, and I > have come across a few minor issues, and one serious bug: > > * The lock-related functions are consistently called MUTEX-foo, but > the macro to grab a mutex is called WITH-LOCK. Does with-mutex-lock sound better? Sam? > * I don't like that (thread-name (make-thread #'(lambda ()))) returns > #<UNBOUND>, as you cannot use (or (thread-name ...) "SOME-DEFAULT") > this way. Why don't you name the thread? I think :name should be mandatory - like for make-mutex and make-exemption. This helps debugging and does not annoy too much. > * I think the default print-object method specializing on threads > should print whether the thread is currently running, or has been > stopped. While it may be useful for quick check - there is thread-active-p for this purpose. (notice that current state of thread (running or finished) may change while you read it - do not rely on it- it is just for debugging). > Now to a much more serious issue: > > * If I run the file below, clisp exits with "Aborted". I'm on > Linux-x86/32, compiled clisp from CVS four hours ago. Have threads > and weak-hashtables been tested? Thanks. weak-hashtables and threads were not tested together. The problem is somehow related to: http://sourceforge.net/tracker/?func=detail&aid=1472478&group_id=1355&atid=101355 While not identical they share quite in common. Threads, mutexes, exemptions (and finalizers) will cause problems when mixed with weak pointers. As for current version of clisp - do not use them together. I'll work on fixing this. Vladimir > PS. > > (defvar *thread-plist-table-lock* > (mp:make-mutex :name "THREAD-PLIST-TABLE-LOCK")) > > (defvar *thread-plist-table* (make-hash-table :weak :key) > "A hashtable mapping threads to a plist.") > > (defvar *thread-id-counter* 0) > > (defun thread-id (thread) > (mp:with-lock (*thread-plist-table-lock*) > (or (getf (gethash thread *thread-plist-table*) 'thread-id) > (setf (getf (gethash thread *thread-plist-table*) 'thread-id) > (incf *thread-id-counter*))))) > > (defvar *thread* (make-thread (lambda () (loop (sleep 1))))) > > (thread-id *thread*) > > (thread-interrupt *thread* :function t) > > (setq *thread* nil) > > (gc) > |
From: Sam S. <sd...@gn...> - 2009-07-12 05:33:34
|
> * Vladimir Tzankov <igmnaxbi@tznvy.pbz> [2009-07-11 23:52:01 +0300]: > > On 7/11/09, Tobias C. Rittweiler <tc...@fr...> wrote: >> >> I tried to use the new thread interface in Clisp's SWANK backend, and I >> have come across a few minor issues, and one serious bug: >> >> * The lock-related functions are consistently called MUTEX-foo, but >> the macro to grab a mutex is called WITH-LOCK. > > Does with-mutex-lock sound better? Sam? WITH-LOCKED-MUTEX sounds like a better English, but, minding APROPOS, your version would probably be better. >> * I don't like that (thread-name (make-thread #'(lambda ()))) returns >> #<UNBOUND>, as you cannot use (or (thread-name ...) "SOME-DEFAULT") >> this way. > > Why don't you name the thread? I think :name should be mandatory - > like for make-mutex and make-exemption. This helps debugging and does > not annoy too much. I think :name should default to Closure_name(function). this way (thread-name (make-thread #'(lambda ()))) would return :LAMBDA another option is for the :name argument to default to a gensym. also, I am not sure I like the requirement that the name should be a string. I think that at least symbols should also be allowed. >> * I think the default print-object method specializing on threads >> should print whether the thread is currently running, or has been >> stopped. > > While it may be useful for quick check - there is thread-active-p for > this purpose. (notice that current state of thread (running or > finished) may change while you read it - do not rely on it- it is just > for debugging). I think prefixing INACTIVE for dead threads might be a good idea. I will do that, unless you object. -- Sam Steingold (http://sds.podval.org/) on Ubuntu 9.04 (jaunty) http://mideasttruth.com http://memri.org http://truepeace.org http://palestinefacts.org http://www.memritv.org will write code that writes code that writes code for food |
From: Vladimir T. <vtz...@gm...> - 2009-07-12 10:51:38
|
On 7/12/09, Sam Steingold <sd...@gn...> wrote: > > WITH-LOCKED-MUTEX sounds like a better English, but, minding APROPOS, > your version would probably be better. changed it to WITH-MUTEX-LOCK >>> * I don't like that (thread-name (make-thread #'(lambda ()))) returns >>> #<UNBOUND>, as you cannot use (or (thread-name ...) "SOME-DEFAULT") >>> this way. >> >> Why don't you name the thread? I think :name should be mandatory - >> like for make-mutex and make-exemption. This helps debugging and does >> not annoy too much. > > I think :name should default to Closure_name(function). > this way (thread-name (make-thread #'(lambda ()))) would return :LAMBDA Done. > also, I am not sure I like the requirement that the name should be a > string. I think that at least symbols should also be allowed. Done (actually the name of the symbol is taken). Also mutex and exemption names are optional now. >>> * I think the default print-object method specializing on threads >>> should print whether the thread is currently running, or has been >>> stopped. >> >> While it may be useful for quick check - there is thread-active-p for >> this purpose. (notice that current state of thread (running or >> finished) may change while you read it - do not rely on it- it is just >> for debugging). > > I think prefixing INACTIVE for dead threads might be a good idea. > I will do that, unless you object. No, I do not object. |
From: Sam S. <sd...@gn...> - 2009-07-12 22:38:36
|
> * Vladimir Tzankov <igmnaxbi@tznvy.pbz> [2009-07-12 13:51:23 +0300]: > > On 7/12/09, Sam Steingold <sd...@gn...> wrote: >> >> also, I am not sure I like the requirement that the name should be a >> string. I think that at least symbols should also be allowed. > > Done (actually the name of the symbol is taken). Also mutex and > exemption names are optional now. I think I was not quite clear on this issue. On a few occasions, I used the following paradigm (not sure this deserves such a grand name): an object would have a slot NAME, and the following invariants would hold: (eq (symbol-value (foo-name foo)) foo) (eq (foo-name (symbol-value sym)) sym) while I am not prepared to extol the virtues of this approach, ISTR that it was quite useful for i/o, data presentation, structural updates (when change-class was inconvenient) &c. I think it would be useful to permit the name slots to be symbols, not just strings. -- Sam Steingold (http://sds.podval.org/) on Ubuntu 9.04 (jaunty) http://palestinefacts.org http://camera.org http://dhimmi.com http://jihadwatch.org http://openvotingconsortium.org http://pmw.org.il There are 10 kinds of people: those who count in binary and those who do not. |
From: Vladimir T. <vtz...@gm...> - 2009-07-13 18:29:11
|
On 7/13/09, Sam Steingold <sd...@gn...> wrote: > I think I was not quite clear on this issue. > > On a few occasions, I used the following paradigm (not sure this > deserves such a grand name): an object would have a slot NAME, > and the following invariants would hold: > > (eq (symbol-value (foo-name foo)) foo) > (eq (foo-name (symbol-value sym)) sym) > > while I am not prepared to extol the virtues of this approach, > ISTR that it was quite useful for i/o, data presentation, structural > updates (when change-class was inconvenient) &c. > > I think it would be useful to permit the name slots to be symbols, not > just strings. Agree. Is it ok to implement it via %record-ref? Vladimir |
From: Sam S. <sd...@gn...> - 2009-07-13 20:22:35
|
Vladimir Tzankov wrote: > Is it ok to implement it via %record-ref? my main thrust was that we must allow at least symbols and integers (for resource pools) as names in addition to strings. I now fixed that. |
From: Vladimir T. <vtz...@gm...> - 2009-07-13 20:58:17
|
On 7/13/09, Sam Steingold <sd...@gn...> wrote: > Vladimir Tzankov wrote: >> Is it ok to implement it via %record-ref? > > my main thrust was that we must allow at least symbols and integers (for > resource pools) as names in addition to strings. > I now fixed that. I meant to use %record-ref in order to make names setf-able. I am not sure that anything is good for name - when setf-able we can easily create circularities: (setf (mutex-name m) m). While there is nothing generally wrong - it should be handled specially when printing and not sure how useful it is. Now: (setf m (make-mutex)) (setf (sys::%record-ref m 0) m) causes stack overflow |
From: Sam S. <sd...@gn...> - 2009-07-13 21:52:59
|
Vladimir Tzankov wrote: > On 7/13/09, Sam Steingold <sd...@gn...> wrote: >> Vladimir Tzankov wrote: >>> Is it ok to implement it via %record-ref? >> my main thrust was that we must allow at least symbols and integers (for >> resource pools) as names in addition to strings. >> I now fixed that. > > I meant to use %record-ref in order to make names setf-able. I am not sure we want them setfable. > I am not sure that anything is good for name - when setf-able we can > easily create circularities: (setf (mutex-name m) m). While there is > nothing generally wrong - it should be handled specially when printing > and not sure how useful it is. well, we can limit the name to... -string -symbol -integer ... how about (make-mutex :name (find-package "CLOS")) ?? > Now: > (setf m (make-mutex)) > (setf (sys::%record-ref m 0) m) > causes stack overflow I think you can avoid it by setting *print-circle* to T. |
From: Vladimir T. <vtz...@gm...> - 2009-07-12 10:56:46
|
On 7/11/09, Vladimir Tzankov <vtz...@gm...> wrote: > weak-hashtables and threads were not tested together. The problem is > somehow related to: > http://sourceforge.net/tracker/?func=detail&aid=1472478&group_id=1355&atid=101355 > While not identical they share quite in common. Threads, mutexes, > exemptions (and finalizers) will cause problems when mixed with weak > pointers. > As for current version of clisp - do not use them together. I'll work > on fixing this. I fixed the issue for MT objects. For finalizers - there is no abort anymore but I am not sure the semantic is correct since the finalizer may "revive" the object. Vladimir |
From: Tobias C. R. <tc...@fr...> - 2009-07-11 22:05:17
|
Vladimir Tzankov <...> writes: > Does with-mutex-lock sound better? Sam? WITH-MUTEX-LOCKED, perhaps? > > * I don't like that (thread-name (make-thread #'(lambda ()))) returns > > #<UNBOUND>, as you cannot use (or (thread-name ...) "SOME-DEFAULT") > > this way. > > Why don't you name the thread? I think :name should be mandatory - > like for make-mutex and make-exemption. This helps debugging and does > not annoy too much. As you say, naming threads is for debuggability. But debugging tools cannot (currently) rely on all threads having names. So it's not about "my" threads at all. I feel mixed about making it mandatory. > > * I think the default print-object method specializing on threads > > should print whether the thread is currently running, or has been > > stopped. > > While it may be useful for quick check - there is thread-active-p for > this purpose. (notice that current state of thread (running or > finished) may change while you read it - do not rely on it- it is just > for debugging). In my case, I looked at MP:LIST-THREADS to see whether the threads stored in a weak hash-table would be reclaimed (they seem not, currently.) I think a thread's status is useful information that should be part of its print representation. > As for current version of clisp - do not use them together. I'll work > on fixing this. Thank you for all your work! -T. |
From: Sam S. <sd...@gn...> - 2009-07-12 05:35:12
|
> * Tobias C. Rittweiler <gpe@serrovgf.qr> [2009-07-11 13:47:45 +0200]: > > (thread-name (make-thread #'(lambda ()))) returns #<UNBOUND> it is certainly a bug for any public function to return #<UNBOUND>. if should either signal an error or return NIL. -- Sam Steingold (http://sds.podval.org/) on Ubuntu 9.04 (jaunty) http://truepeace.org http://openvotingconsortium.org http://dhimmi.com http://palestinefacts.org http://thereligionofpeace.com http://pmw.org.il The paperless office will become a reality soon after the paperless toilet. |
From: Tobias C. R. <tc...@fr...> - 2009-07-12 16:03:07
|
Vladimir Tzankov <vtz...@gm...> writes: > I fixed the issue for MT objects. > For finalizers - there is no abort anymore but I am not sure the > semantic is correct since the finalizer may "revive" the object. The test case of my OP still aborts; even though not each time, but non-deterministically each 5th time. -T. PS. for i in `seq 1 100`; do clisp-cvs /tmp/test.lisp || break done |
From: Vladimir T. <vtz...@gm...> - 2010-02-27 18:33:17
|
Hi Tobias, Somehow I managed to miss this mail half an year ago - sorry. Just today I encountered the problem and saw you comment in swank-clisp.lisp. It's fixed in the cvs. Thanks. Vladimir On 7/12/09, Tobias C. Rittweiler <tc...@fr...> wrote: > Vladimir Tzankov <vtz...@gm...> writes: > >> I fixed the issue for MT objects. >> For finalizers - there is no abort anymore but I am not sure the >> semantic is correct since the finalizer may "revive" the object. > > The test case of my OP still aborts; even though not each time, but > non-deterministically each 5th time. > > -T. > > PS. > > for i in `seq 1 100`; do > clisp-cvs /tmp/test.lisp || break > done |
From: Tobias C. R. <tc...@fr...> - 2010-02-27 23:17:35
|
Vladimir Tzankov <vtz...@gm...> writes: > Hi Tobias, > Somehow I managed to miss this mail half an year ago - sorry. > Just today I encountered the problem and saw you comment in swank-clisp.lisp. > It's fixed in the cvs. > Thanks. > Vladimir Thanks that's good to know. If I find the time, I'll try the swank threading stuff with HEAD. -T. |