www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 4621] New: Destructors are inherently un- safe

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

           Summary: Destructors are inherently un- safe
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: michel.fortin michelf.com


--- Comment #0 from Michel Fortin <michel.fortin michelf.com> 2010-08-11
14:08:33 EDT ---
Accessing the GC heap through a member in a destructors is inherently unsafe
because the GC might have already freed that memory. So destructors in SafeD
should not be able to access the GC-heap through a member. Here is an example:

 safe:

class C {
    C other;
    ~this() {
        writeln(other.toString()); // "other" might already have been freed.
    }
}

void main() {
    C c1 = new C;
    C c2 = new C;
    c1.other = c2;
    c2.other = c1; // creating a circular reference
}

Given that the compiler has no way to know if a reference, pointer, or array
points to the GC heap or elsewhere, it might have to disallow any dereferencing
of any member and calls to functions that might dereference a member. And at
this point you can't do anything useful in a destructor, so you might just
disallow  safe destructors altogether.

Note that this applies to struct destructors too, since structs can be on the
heap (in their own memory block, part of an array, or as a member of a class).

-- 
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=4621


Jonathan M Davis <jmdavisProg gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg gmail.com


--- Comment #1 from Jonathan M Davis <jmdavisProg gmail.com> 2010-08-11
11:36:33 PDT ---
What about structs which are on the stack? I agree that the stuff on the heap
has this problem, but structs on the stack should be fine, shouldn't they?

I'd hate for structs on the stack not be able to have destructors except in
SafeD. It would make RAII only work in SafeD, which would not be good.

I do agree that destructors on the heap should be disallowed in SafeD, but I
don't want to see structs on the heap not being allowed to have destructors in
SafeD.

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



--- Comment #2 from Michel Fortin <michel.fortin michelf.com> 2010-08-11
14:37:52 EDT ---
Perhaps a better solution for structs: have a way to distinguish between a
struct that can be put on the GC heap and one that cannot. A struct that cannot
go on the GC heap make it safe to access GC-managed members in its destructor,
and thus can have a  safe destructor.

But at the same time such a struct would be prohibited at compile time from
being part of a class, or from being allocated with "new" (either solitary or
part of an array).

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



--- Comment #3 from Jonathan M Davis <jmdavisProg gmail.com> 2010-08-11
11:43:00 PDT ---
Ouch. That last sentence of my needs editing. I meant to say that I don't want
to see structs on the _stack_ not being allowed to have destructors in SafeD.
But obviously you understood what I meant.

As for your suggestion, couldn't the compiler just disallow structs with
destructors from anywhere but the stack in SafeD? If you try and declare them
anywhere else, you'd get an error. There shouldn't be any need to distinguish
them otherwise. The destructor itself could do that.

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



--- Comment #4 from Michel Fortin <michel.fortin michelf.com> 2010-08-11
15:33:11 EDT ---
The problem with structs is that many structs will need a destructor because
they encapsulate a resource not managed by the GC. That destructor can be made
GC-heap safe, and thus this struct can be put in a class. For instance, a File
struct wrapping a file handle could easily be made GC-heap safe if it's
destructor just calls fclose(handle), and thus could be put in a class. Are you
willing to make this File struct unusable in SafeD? Or std.container.Array,
which is totally safe to use on the heap too?

So I think there is a need to distinguish GC-safe structs from those that
aren't. Forbidding all structs with a destructor to be put on the heap in SafeD
is going to prevent too many useful things. Obviously, the struct itself would
need a  trusted destructor.

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



--- Comment #5 from Jonathan M Davis <jmdavisProg gmail.com> 2010-08-11
13:10:28 PDT ---
This mess is just too complicated. Sigh. Well, we want to be able to use
structs with destructors in SafeD wherever is reasonable to use them, and those
uses should be allowed. Unsafe uses should not be allowed. If attributes of
some kind are required on the structs or their destructors to make it work,
then that's the path that we should take.

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



--- Comment #6 from Michel Fortin <michel.fortin michelf.com> 2010-08-11
16:20:44 EDT ---
It could be an attribute, or it could be something else.

For instance, instead of having just destructors, we could have destructors
(~this) and finalizers (~~this). A struct with neither can go anywhere, a
struct with a destructor but no finalizer cannot go on the GC-heap,  a struct
with only a finalizer can go anywhere (the finalizer is used as the
destructor), and a struct with both can go anywhere. The finalizer cannot be
made  safe.

Doing this with structs would probably mean allowing only finalizers (~~this)
for classes, which according to my syntax suggestion would break existing code
for class destructors. Perhaps the syntax should be different.

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


Steven Schveighoffer <schveiguy yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy yahoo.com


--- Comment #7 from Steven Schveighoffer <schveiguy yahoo.com> 2010-08-12
05:21:10 PDT ---
(In reply to comment #6)
 It could be an attribute, or it could be something else.
 
 For instance, instead of having just destructors, we could have destructors
 (~this) and finalizers (~~this). A struct with neither can go anywhere, a
 struct with a destructor but no finalizer cannot go on the GC-heap,  a struct
 with only a finalizer can go anywhere (the finalizer is used as the
 destructor), and a struct with both can go anywhere. The finalizer cannot be
 made  safe.
I think rather than prevent where these items should go, you should just not call the destructor when the struct/class is being destroyed by the GC. I'd say you could even prevent what the finalizer contains, but it's too limiting for the compiler to assume what type of memory a reference is referencing. My thought is simply that a finalizer is not safe, and a destructor is safe. That at least gives a path for implementation (just mark your finalizer as trusted, and it can be used in SafeD). -- 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=4621


nfxjfg gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nfxjfg gmail.com


--- Comment #8 from nfxjfg gmail.com 2010-08-12 06:20:27 PDT ---
*facepalm*

C# and Java have safe finalizers. There's nothing that stops you rewriting the
finalization part in gcx.d to make it safe. The only real issue is that
finalizers are not deterministic (unsolvable) and that finalizers are invoked
in an arbitrary context (solvable if you'd create a finalization worker thread
or so).

-- 
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=4621



--- Comment #9 from Michel Fortin <michel.fortin michelf.com> 2010-08-12
09:37:20 EDT ---
(In reply to comment #7)
 I think rather than prevent where these items should go, you should just not
 call the destructor when the struct/class is being destroyed by the GC.
One thing is that I fear it becomes too easy to write a destructor and then forget the finalizer, which would results in leaks everywhere. So I think there has to be something to remind people that they need a finalizer when they need to dispose of some resource and the struct is allocated on the heap. But just not calling the finalizer is still a better option than calling the destructor during collection: a leak is less harmful than memory corruption. Forbidding GC-allocated structs could be useful to make some RAII patterns safer too. For instance, think of a struct representing a database transaction: it should probably live on the stack, you certainly don't want it to be garbaged collected. The compiler enforcing deterministic destruction would improve program correctness for this kind of thing.
 I'd say you could even prevent what the finalizer contains, but it's too
 limiting for the compiler to assume what type of memory a reference is
 referencing.  My thought is simply that a finalizer is not safe, and a
 destructor is safe.  That at least gives a path for implementation (just mark
 your finalizer as  trusted, and it can be used in SafeD).
Whatever it is, I'm now of the opinion that the limitation should only apply to SafeD. The limitation could be about dereferencing members and calling functions that might dereference a member, or it could be that the finalizer itself is not allowed to be safe. Allowing non-dereferencing actions in the finalizer in SafeD would still allow a few things, such as printing a "Hello I'm finalized!" message on the console or updating a global variable (such as a counter of live objects). Nothing very interesting though. But it'd also allow an empty finalizer to be safe, which might be useful if the presence of a destructor and the absence of a finalizer prevents allocation on 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=4621



--- Comment #10 from Steven Schveighoffer <schveiguy yahoo.com> 2010-08-12
06:43:37 PDT ---
(In reply to comment #8)
 C# and Java have safe finalizers.
According to the few pages I googled for C# finalizers, this is not true. You are not supposed to access/attempt to destroy GC managed resources. In fact, C# doesn't even allow manual deletion of such resources. The only purpose of finalizers is to clean up non-GC resources. After reading those pages, it looks like C# is almost identical in the restrictions and guarantees as D is. -- 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=4621



--- Comment #11 from Michel Fortin <michel.fortin michelf.com> 2010-08-12
09:54:51 EDT ---
(In reply to comment #8)
 C# and Java have safe finalizers.
Indeed. Java allows resurrection, which means that if you leak a reference to an object during the collection, the GC won't collect that object and will leave it in the state it was after the finalizer was called. This is an interesting idea, but I see two reasons it'll not work fro D. First, D doesn't emit special code notifying the GC when assigning to a pointer, so the GC would have to do a full scan again after a collection just to check if someone resurrected a memory block. The second reason is the D2 multithreading model. The GC might run on a different thread. If you resurrect a non-shared object, that object won't live on the same thread anymore but might continue to reference non-shared memory from other threads. This is in violation of the thread-safe type system. -- 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=4621



--- Comment #12 from nfxjfg gmail.com 2010-08-12 07:06:10 PDT ---
By the way, separation between finalizers and destructors has been in Tango for
ages.

There's the Object.dipose() method. This method is only called on deterministic
destruction, e.g on delete or with scope classes.

The "destructor" ~this is the finalizer and is always called, both on delete or
on collection.

(This was done so mainly for backward comnpatibility, while still satisfying
the need for knowing about deterministic destruction.)

-- 
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=4621



--- Comment #13 from nfxjfg gmail.com 2010-08-12 07:10:27 PDT ---
(In reply to comment #11)
 This is an interesting idea, but I see two reasons it'll not work fro D. First,
 D doesn't emit special code notifying the GC when assigning to a pointer, so
 the GC would have to do a full scan again after a collection just to check if
 someone resurrected a memory block.
This isn't so much of a problem if you assume objects with finalizers are rare. They can be collected in the next GC cycle.
 The second reason is the D2 multithreading model. The GC might run on a
 different thread. If you resurrect a non-shared object, that object won't live
 on the same thread anymore but might continue to reference non-shared memory
 from other threads. This is in violation of the thread-safe type system.
Good point. By definition, if the object is not shared(), the finalizer (or anything) must not run on a different thread. It doesn't matter if you access references or not. I wonder how D2 can have finalizers at all with this. -- 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=4621



--- Comment #14 from Michel Fortin <michel.fortin michelf.com> 2010-08-12
10:46:23 EDT ---
(In reply to comment #13)
 This isn't so much of a problem if you assume objects with finalizers are rare.
 They can be collected in the next GC cycle.
I'm not sure we can make this assumption. Beside, is it worth it? I mean, what is the use of a resurrected object beyond providing a little more safety? Given the multithreading model, we know finalizers can't be made safe; them being half-safer doesn't bring us much.
 Good point. By definition, if the object is not shared(), the finalizer (or
 anything) must not run on a different thread. It doesn't matter if you access
 references or not. I wonder how D2 can have finalizers at all with this.
Well, the object's memory block itself is no longer referenced by other threads (otherwise it would not be collected), so I guess as long as you only access values inside this memory block you're safe. You can probably also access non-GC memory you're the sole owner of, or non-GC shared memory. But you shouldn't access non-shared globals, or non-shared memory that someone else could have a reference to. This is starting to be really complicated, but except for the non-GC memory part it looks quite similar to the restrictions applied to methods of a synchronized class. -- 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=4621



--- Comment #15 from nfxjfg gmail.com 2010-08-12 08:10:19 PDT ---
(In reply to comment #14)
 Beside, is it worth it? I mean, what is the use of a resurrected object beyond
 providing a little more safety? Given the multithreading model, we know
 finalizers can't be made safe; them being half-safer doesn't bring us much.
If you'd move finalizer execution to a dedicated finalizer thread, it'd be already quite safe. Then you can acquire locks to do synchronized access to your data. But it still doesn't fit in D2's typesystem.
 Well, the object's memory block itself is no longer referenced by other threads
 (otherwise it would not be collected), so I guess as long as you only access
 values inside this memory block you're safe. You can probably also access
 non-GC memory you're the sole owner of, or non-GC shared memory. But you
 shouldn't access non-shared globals, or non-shared memory that someone else
 could have a reference to. This is starting to be really complicated, but
 except for the non-GC memory part it looks quite similar to the restrictions
 applied to methods of a synchronized class.
Running on a different thread still makes a severe difference to shared or C data. C APIs usually aren't thread-safe. For some OS APIs, the caller thread makes a difference (for instance, you'd break OS provided TLS). You'll have to make an explicit exception in the language spec for finalizers to allow this. -- 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=4621



--- Comment #16 from Michel Fortin <michel.fortin michelf.com> 2010-08-12
12:30:40 EDT ---
(In reply to comment #15)
 Running on a different thread still makes a severe difference to shared or C
 data. C APIs usually aren't thread-safe. For some OS APIs, the caller thread
 makes a difference (for instance, you'd break OS provided TLS).
This depends on the meaning of shared vs. non-shared. Is a non-shared object guarantied to always exist in the same thread? Or is it only guarantied to be accessible in one thread at a time? The former would forbid objects with a unique reference to a non-shared memory block from being moved from one thread to another with no copying, so I think the later is more useful.
 You'll have to make an explicit exception in the language spec for finalizers
 to allow this.
With the "accessible in one thread at a time" model, the only additional special case with multithreading is that you can't access non-shared memory referenced by a member if there's a chance this memory might still be in referenced by the original thread. Combine this with the problem of GC-managed memory which could have already be deallocated and multithreading only complicates the case where you have manually managed memory shared between objects (such as reference counting)... ... and I think I've found such a bug in std.containers.Array (added to bug 4624). -- 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=4621



--- Comment #17 from Michel Fortin <michel.fortin michelf.com> 2010-08-31
07:27:54 EDT ---
In the event keeping a combined destructor-finalizer is the favored option,
this could be done by repurposing the to-be-deprecated "scope" qualifier.
"scope" could be applied as an attribute to structs and classes and would
prevent the the struct/class from being allocated on the GC-heap. The absence
of "scope" would make the destructor a finalizer and dereferencing a member
would be prohibited in it without some kind of cast.

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


Maxim Fomin <maxim maxim-fomin.ru> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |maxim maxim-fomin.ru
         Resolution|                            |INVALID


--- Comment #18 from Maxim Fomin <maxim maxim-fomin.ru> 2013-08-05 13:21:52 PDT
---
This is invalid report since  safe has nothing to do with accessing
pointers/references which turned out to be nulls. This is valid D code:

void foo(int* p)  safe // or ref
{
   *p = 0;
} 

void main()  safe
{
   foo(null);
}

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



--- Comment #19 from Michel Fortin <michel.fortin michelf.com> 2013-08-05
17:13:32 EDT ---
(In reply to comment #18)
 This is invalid report since  safe has nothing to do with accessing
 pointers/references which turned out to be nulls.
Maxim, you're the first to mention null here. I'm not sure I get your point. This issue is about accessing destructed/deallocated memory from the destructor while GC is finalizing an object (or a struct on the GC heap). This can happen if you have a circular reference for instance, or anytime multiple objects that references themselves are finalized in the same pass. The most evil thing you could do is to leak a reference to the a finalized object to the outer world, and then you have a pointer to deallocated memory lying around somewhere. There's no way to catch any of this (currently), hence why destructors are unsafe (when called from the GC). -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Aug 05 2013