From: Jean-Claude Beaudoin on

I have been reading the CLOS code of ECL lately,
a somewhat far derivative of PCL I think, and I
realized that all that metaobject business was
quite clearly NOT thread-safe. Classes, methods
and generic functions are happily modified
outside of any lock.

The implementation notes of clisp currently state
also that: "CLOS is NOT thread-safe". So the
situation seems to be somewhat common among CL
implementations.

That cannot be too much of a good thing in
these days of multi-core CPUs and it bothers
me quite a bit! So I'd like to fix it, at least
in the implementation I currently use (ECL).

How difficult can it be to make CLOS thread-safe?
Does any of you have an (informed?) opinion on that?

I saw that SBCL wraps a good number of CLOS
functions in a "with-world-lock". Is that a
sound approach? Wouldn't a more focused
"with-metadata-lock" be good enough instead
of locking the whole world?

CLOS feels like it has so many customizable hooks
that the potential for deadlock through some
of that customization code is quite great.
In that context a clearly stated and widely
publicized metadata locking policy would have
to be established. Am I wrong on this?

Also, make-instance and its "two step"
process feels like a source of troubles.
First call generic-function "allocate-instance"
and then call generic-function "initialize-instance";
what about the void between these two?
I would hate to have to grab a lock to
instantiate objects!

Any advice on this whole subject would be most appreciated.

Thanks,

Jean-Claude Beaudoin
From: D Herring on
On 08/10/2010 12:10 AM, Jean-Claude Beaudoin wrote:
> Any advice on this whole subject would be most appreciated.

The following comments are an initial reaction to reading your post.
i.e. not well thought out.


> I have been reading the CLOS code of ECL lately,
> a somewhat far derivative of PCL I think, and I
> realized that all that metaobject business was
> quite clearly NOT thread-safe. Classes, methods
> and generic functions are happily modified
> outside of any lock.
....
> How difficult can it be to make CLOS thread-safe?
> Does any of you have an (informed?) opinion on that?
>
> I saw that SBCL wraps a good number of CLOS
> functions in a "with-world-lock". Is that a
> sound approach? Wouldn't a more focused
> "with-metadata-lock" be good enough instead
> of locking the whole world?

SBCL's approach seems like a good one. It is relatively simple and
effective.

I imagine SBCL achieves the sync by using the same mechanism it uses
for GC. Stop the world. At that point, there is no benefit to using
a different name. This is slow when it happens; but it removes the
need for conditional checks during normal runtime.


> CLOS feels like it has so many customizable hooks
> that the potential for deadlock through some
> of that customization code is quite great.
> In that context a clearly stated and widely
> publicized metadata locking policy would have
> to be established. Am I wrong on this?

As long as all synchronization uses the same CLOS-mutation lock, and
the code interior to CLOS doesn't grab any other locks, its probably
deadlock-free. If the CLOS mutex is always the last to be grabbed,
then it guarantees a proper partial order. A fine-grain system with
multiple locks or lock-free algorithms would require a better
specification.


> Also, make-instance and its "two step"
> process feels like a source of troubles.
> First call generic-function "allocate-instance"
> and then call generic-function "initialize-instance";
> what about the void between these two?
> I would hate to have to grab a lock to
> instantiate objects!

That is indeed something to fear. I don't see an obvious solution.
Any function which sequences CLOS calls could be affected. Stuff like
this argues for a sort of immutable structure system, something that
allows each thread to use a consistent set of bindings until its stack
frame returns past some boundary. Pascal C. has illustrated the use
of ContextL for cleanly solving such issues. Don't know if it was
truly thread-safe, though.

- Daniel
From: Kenneth Tilton on
On 8/10/2010 8:28 PM, D Herring wrote:
> On 08/10/2010 12:10 AM, Jean-Claude Beaudoin wrote:
>> Any advice on this whole subject would be most appreciated.
>
> The following comments are an initial reaction to reading your post.
> i.e. not well thought out.
>
>
>> I have been reading the CLOS code of ECL lately,
>> a somewhat far derivative of PCL I think, and I
>> realized that all that metaobject business was
>> quite clearly NOT thread-safe. Classes, methods
>> and generic functions are happily modified
>> outside of any lock.
> ...
>> How difficult can it be to make CLOS thread-safe?
>> Does any of you have an (informed?) opinion on that?
>>
>> I saw that SBCL wraps a good number of CLOS
>> functions in a "with-world-lock". Is that a
>> sound approach? Wouldn't a more focused
>> "with-metadata-lock" be good enough instead
>> of locking the whole world?
>
> SBCL's approach seems like a good one. It is relatively simple and
> effective.
>
> I imagine SBCL achieves the sync by using the same mechanism it uses for
> GC. Stop the world. At that point, there is no benefit to using a
> different name. This is slow when it happens; but it removes the need
> for conditional checks during normal runtime.
>
>
>> CLOS feels like it has so many customizable hooks
>> that the potential for deadlock through some
>> of that customization code is quite great.
>> In that context a clearly stated and widely
>> publicized metadata locking policy would have
>> to be established. Am I wrong on this?
>
> As long as all synchronization uses the same CLOS-mutation lock, and the
> code interior to CLOS doesn't grab any other locks, its probably
> deadlock-free. If the CLOS mutex is always the last to be grabbed, then
> it guarantees a proper partial order. A fine-grain system with multiple
> locks or lock-free algorithms would require a better specification.
>
>
>> Also, make-instance and its "two step"
>> process feels like a source of troubles.
>> First call generic-function "allocate-instance"
>> and then call generic-function "initialize-instance";
>> what about the void between these two?
>> I would hate to have to grab a lock to
>> instantiate objects!
>
> That is indeed something to fear. I don't see an obvious solution. Any
> function which sequences CLOS calls could be affected. Stuff like this
> argues for a sort of immutable structure system, something that allows
> each thread to use a consistent set of bindings until its stack frame
> returns past some boundary. Pascal C. has illustrated the use of
> ContextL for cleanly solving such issues. Don't know if it was truly
> thread-safe, though.
>
> - Daniel

Sorry, what exactly is the problem? A relevant method being defined
between allocate-instance and initialize-instance calls on one instance
in a way that matters?

What we do in my shop is take that monkey out back and shoot them. PETA
gives us hell, but it does wonders for discipline.

hth, kt

--
http://www.stuckonalgebra.com
"The best Algebra tutorial program I have seen... in a class by itself."
Macworld
From: Tim Bradshaw on
On 2010-08-11 01:28:49 +0100, D Herring said:
>
> SBCL's approach seems like a good one. It is relatively simple and effective.

If this world-lock means "stop all other mutation in the system", and
if it happens significntly often, then Amdahl's law will be hurting you.

It seems to me that it would be reasonable for the system to protect
things which (for instance) create new classes and so on, but for (for
instance) instance creation then it's really up to you, since CLOS
can't know what methods you define. And even then there are so many
places where user code can intervene that it's hard to see how much
useful could be done.

There are clearly cases where the system needs to at least ensure
things are safe. For instance I think that any serious implementation
would be caching effective methods, and that cache needs to be safe
(it's OK for multiple things to think they need to recompute the
method).

I agree with Kenny that it's not realistic to expect things to be safe
in the presence of (for instance) new methods being defined half way
through the computation of an effective method: if your program does
that, you deserve to lose.

From: Jean-Claude Beaudoin on

I missed the fact that there is a third generic function involved in the business of make-instance: shared-initialize.
So it makes for at least two (if not three) holes through which trouble can creep in. First between allocate-instance
and instance-initialize and second between the beginning of instance-initialize and its own invocation of shared-initialize.


Kenneth Tilton wrote:
>
> Sorry, what exactly is the problem? A relevant method being defined
> between allocate-instance and initialize-instance calls on one instance
> in a way that matters?
>

Currently I see at least two problematic scenarios.

The first one is in the event of a change in the DAG of classes that happen to modify the class precedence list (CPL) of
a class for which a make-instance is in progress. I don't have much hope for the coherence of the whole set of
allocate-instance, instance-initialize and shared-initialize if they have been customized with code that share some
critical assumptions about the class they work on. But also in this situation you will end up in a call to generic
function update-instance-for-redefined-class right in the middle of the on-going make-instance and this in the context
of a partially or totally uninitialized instance. I doubt this is a context that update-instance-for-redefined-class is
likely to handle gracefully. The potential for all of this to end in a call to the debugger is much too large for me to
be happy about it. I think a solution here would be to defer somehow the instance updating to a point after the
initialization is done.

The second scenario is a call to make-instance between a (defmethod allocate-instance ...) and a (defmethod
instance-initialize ...) or vice-versa and again allocate-instance and instance-initialize share some critical
assumption about the class to instantiate. Caching as one single unit of the 3 effective methods involved in the
execution of make-instance is probably a solution to this one, the cache line being filled under the protection of the
metadata lock.

For the rest of the CLOS metadata modifying code (defclass, defmethod, defgeneric, ...) I am about to do pretty much
like SBCL and wrap most of it in a bunch of with-metadata-lock...

Cheers,

Jean-Claude Beaudoin