clisp-devel

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)

On 7/11/09, Tobias C. Rittweiler <tcr@...> 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)
>

> * Vladimir Tzankov <igmnaxbi@...> [2009-07-11 23:52:01 +0300]:
>
> On 7/11/09, Tobias C. Rittweiler <tcr@...> 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.comhttp://memri.orghttp://truepeace.orghttp://palestinefacts.orghttp://www.memritv.org
will write code that writes code that writes code for food

On 7/12/09, Sam Steingold <sds@...> 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.

> * Vladimir Tzankov <igmnaxbi@...> [2009-07-12 13:51:23 +0300]:
>
> On 7/12/09, Sam Steingold <sds@...> 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.orghttp://camera.orghttp://dhimmi.comhttp://jihadwatch.orghttp://openvotingconsortium.orghttp://pmw.org.il
There are 10 kinds of people: those who count in binary and those who do not.

On 7/13/09, Sam Steingold <sds@...> 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

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.

On 7/13/09, Sam Steingold <sds@...> 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

Vladimir Tzankov wrote:
> On 7/13/09, Sam Steingold <sds@...> 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.

On 7/11/09, Vladimir Tzankov <vtzankov@...> 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

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.

Vladimir Tzankov <vtzankov@...> 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

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 <tcr@...> wrote:
> Vladimir Tzankov <vtzankov@...> 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

Vladimir Tzankov <vtzankov@...> 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.

Community

Help

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

I agree to receive quotes, newsletters and other information from sourceforge.net and its partners regarding IT services and products. I understand that I can withdraw my consent at any time. Please refer to our Privacy Policy or Contact Us for more details