www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 4624] New: std.stdio.File and std.typecons.Unique not GC-heap safe

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4624

           Summary: std.stdio.File and std.typecons.Unique not GC-heap
                    safe
           Product: D
           Version: D2
          Platform: Other
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: nobody puremagic.com
        ReportedBy: michel.fortin michelf.com



15:28:53 EDT ---
Just took a look at Phobos for struct destructors. Both std.stdio.File and
std.typeconst.Unique seem unsafe to store anywhere in the GC heap (in an array
or in a class).

For std.stdio.File, it's because because the destructor assumes the
GC-allocated Impl instance can be dereferenced (which is a risky bet during the
collection that the GC will deallocate things in the "right" order):

    this(string name, in char[] stdioOpenmode = "rb")
    {
        p = new Impl(errnoEnforce(.fopen(name, stdioOpenmode),
                        "Cannot open file `"~name
                        ~"' in mode `"~stdioOpenmode.idup~"'"),
                1, name);
    }

    ~this()
    {
        if (!p) return;
        //    BUG    These lines prematurely close the file
        //printf("Destroying file `%s' with %d refs\n", toStringz(p.name),
p.refs);
        if (p.refs == 1) close;
        else --p.refs;
    }

In struct std.typecons.Unique(T), unique calls delete on the object it
references, but since that object is in the GC heap the same problem arises: it
might already have been deallocated:

    ~this()
    {
        writeln("Unique destructor of ", (_p is null)? null: _p);
        delete _p;
        _p = null;
    }

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 11 2010
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4624




12:29:25 EDT ---
Additionally, I believe std.containers.Array has a race condition when stored
in the GC heap. The Array destructor checks the reference count before deciding
whether it should free the array's content or not. This reference counter is
non-shared, but the collection cycle can run on any thread so there is a race
when accessing the reference counter.

    private struct Data
    {
        uint _refCount = 1;
        size_t _capacity;
        T[] _payload;
        this(T[] p) { _capacity = p.length; _payload = p; }
    }

The reference counter should be made shared so it is only accessed using atomic
operations. Other fields don't need to be shared because the only time they're
accessed in the destructor is when the reference counter falls to zero and the
destructor has the only remaining reference.

This problem is also present in std.stdio.File (in addition to the other
problem above) and std.typecons.RefCounted which both use a non-shared
reference counter, rendering them unsuitable for being put in the GC heap.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 12 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4624




Commit pushed to master at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/26b7d3fad37c2cd8e592f4ab4021ba014ff35bd0
Fix std.stdio.File part of Issue 4624

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 04 2012
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=4624




14:13:23 EDT ---


 Commit pushed to master at https://github.com/D-Programming-Language/phobos
 
 https://github.com/D-Programming-Language/phobos/commit/26b7d3fad37c2cd8e592f4ab4021ba014ff35bd0
 Fix std.stdio.File part of Issue 4624


This only fixes one side of the std.stdio.File part of this issue: there's
still a race between the collecting thread and other threads when accessing and

issue). The reference counter should only be changed using atomic
increment/decrement to prevent that race, at least until the GC can guaranty it
will collect objects in the same thread they were created in.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Oct 04 2012