On Sat, Aug 2, 2014 at 6:00 AM, Nick Wellnhofer <[email protected]> wrote:
> On Aug 1, 2014, at 18:34 , Marvin Humphrey <[email protected]> wrote:
>
>> It would be great if we could replace To_Host() with an ordinary subroutine
>> and remove it as a method from Obj.
>
> Can we postpone this and simply remove the void* mapping for now?

No objections here.  Thanks for digging up that snippet!

>> (Hmm, it occurs to me that other global classes such as Clownfish::Method may
>> also need SvSHARE enabled.  Also, using non-threadsafe classes such as String
>> and VArray within Class and Method is probably a bug.)
>
> Yes, it looks like Clownfish::Method needs SvSHARE. The use of
> non-threadsafe classes shouldn’t be a problem if data is modified only
> during initialization. Afterwards, Class and Method should be treated as
> immutable.

Agreed about mutability being OK during initalization, but what I'm referring
to are member variables:

*   Clownfish::Class => "name" (String)
*   Clownfish::Class => "methods" (VArray)
*   Clownfish::Method => "name" (String)
*   Clownfish::Method => "host_alias" (String)

Under refcounting hosts, the refcounts of all of those objects are vulnerable
to race bugs under multi-threading.

Under Perl we also have the cached host object to think about; that won't be a
concern under e.g. Python because Clownfish objects will inherit from Python's
`object` rather than cache a host object.  Perl is probably an odd case here.

Since the refcounts on Class and Method objects are held constant, they are
not themselves vulnerable to refcount race conditions -- but that's not true
for member variables whose refcounts are not held constant.

In addition, the "methods" VArray is a concern because unlike the Strings, its
content is mutable.

I think the most straightforward way to address this issue would be to use
raw arrays instead of Clownfish types, with `char*` in place of String
and `Method**` in place of the VArray.  That would be annoying, though,
for frequently accessed members like `name`.

Perhaps a better option would be to control refcounting with a flag.

        if (!(obj->ref.count & CFISH_OBJ_IS_IMMORTAL)) {
            ...
        }

If we do that, then we don't need refcounting to go through method calls any
more.  The only reason we've ever needed to override the refcounting methods
is to thwart refcounting (Class, Method, HashTombStone, RawPosting).

Marvin Humphrey

Reply via email to