www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Segmentation fault on closing file in destructor

reply Tom Kazimiers <2voodoo gmx.de> writes:
Hi,

a file reading class of mine can be constructed with a filename as
parameter. It instantiates a new std.stream.File (without the passed
file name and closes it when opened within the destructor. The last part
is where things are getting unclear for me. On the "file.isOpen()" call
in the destructor a segmentation fault occurs. What is the problem with
that?

Thanks in advance,
Tom

An example class to demonstrate the problem:

import std.stdio;
import std.string;
import std.stream;

class FileReader(T) {
    string          filename; // the name of the object file
    std.stream.File file;     // the object file

  public:
    this(string filename) {
        // make an immutable copy of the filename
        this.filename = filename.idup;
        file = new std.stream.File();
    }

    // destructor
    ~this() {
        if(file.isOpen())  // <---- crashes here
            writefln("open");
    }
	
    // open file, return true if successful
    bool open() {
        try {
            file = new std.stream.File(filename, FileMode.In);
        } catch(OpenException e) {
            writefln("[Error] Could not open file: " ~ filename);
            return false;
        }
        return true;
    }
}
Sep 26 2010
next sibling parent reply "Simen kjaeraas" <simen.kjaras gmail.com> writes:
Tom Kazimiers <2voodoo gmx.de> wrote:

 Hi,

 a file reading class of mine can be constructed with a filename as
 parameter. It instantiates a new std.stream.File (without the passed
 file name and closes it when opened within the destructor. The last part
 is where things are getting unclear for me. On the "file.isOpen()" call
 in the destructor a segmentation fault occurs. What is the problem with
 that?

Likely, it is this[1]: "[T]he order in which the garbage collector calls destructors for unreference objects is not specified. This means that when the garbage collector calls a destructor for an object of a class that has members that are references to garbage collected objects, those references may no longer be valid. This means that destructors cannot reference sub objects." [1]: http://digitalmars.com/d/2.0/class.html#destructors -- Simen
Sep 26 2010
next sibling parent reply bearophile <bearophileHUGS lycos.com> writes:
Tom Kazimiers:

 Do you have any suggestion how I should make
 sure that the file gets closed on destruction?

Can you use scope(exit) or the std.stdio.File? Bye, bearophile
Sep 26 2010
parent Tom Kazimiers <2voodoo gmx.de> writes:
Hi,

On 09/26/2010 10:05 PM, Simen kjaeraas wrote:
 Can you use scope(exit) or the std.stdio.File?

Well, I have no idea. You mentioning scope(exit) was actually the first time I heard of it. Unfortunately I have not found any resource about it. Do you have a link to point me in the right direction?

http://digitalmars.com/d/2.0/statement.html#ScopeGuardStatement

thanks, strange I did not find that page by searching. Good to know about that concept, but I guess it is not fitting as a solution here. My scope is the from creation to destrution of the class, so this will no be working. But anyway, thanks. Tom
Sep 28 2010
prev sibling next sibling parent Tom Kazimiers <2voodoo gmx.de> writes:
Hi,

On 09/26/2010 07:13 PM, bearophile wrote:
 Tom Kazimiers:
 
 Do you have any suggestion how I should make
 sure that the file gets closed on destruction?

Can you use scope(exit) or the std.stdio.File?

Well, I have no idea. You mentioning scope(exit) was actually the first time I heard of it. Unfortunately I have not found any resource about it. Do you have a link to point me in the right direction? If I would use std.stdio.File, what would be different? Thanks, Tom
Sep 26 2010
prev sibling next sibling parent =?UTF-8?B?IkrDqXLDtG1lIE0uIEJlcmdlciI=?= <jeberger free.fr> writes:
Tom Kazimiers wrote:
 Hi Simen,
 
 On 09/26/2010 04:06 PM, Simen kjaeraas wrote:
 Likely, it is this[1]:

 "[T]he order in which the garbage collector calls destructors for
 unreference objects is not specified. This means that when the garbage
 collector calls a destructor for an object of a class that has members
 that are references to garbage collected objects, those references may
 no longer be valid. This means that destructors cannot reference sub
 objects."

 [1]: http://digitalmars.com/d/2.0/class.html#destructors

thanks for your reply. I did not know that the garbage collector works that way. Is the reason for that flexibility for the GC? What if I want to handle some destruction parts to sub-objects or if I want conditions while destruction, based on sub-objects? Or maybe I should generally avoid situations like that. Do you have any suggestion how I should make sure that the file gets closed on destruction?

1. You really need precise control as to when the file is closed. In that case, your class contains an explicit cleanup function that you call before dropping the last reference and you can close the file at that time; 2. You only need to be sure that the file gets closed at some point but it doesn't really signify when. In that case, you let the GC collect your class and you don't close the file. Eventually the GC will collect the std.stream.File instance which will result in calling its destructor which will close the file without your needing to do anything about it. Jerome -- mailto:jeberger free.fr http://jeberger.free.fr Jabber: jeberger jabber.fr
Sep 26 2010
prev sibling next sibling parent Michel Fortin <michel.fortin michelf.com> writes:
On 2010-09-26 10:06:33 -0400, "Simen kjaeraas" <simen.kjaras gmail.com> said:

 Tom Kazimiers <2voodoo gmx.de> wrote:
 
 Hi,
 
 a file reading class of mine can be constructed with a filename as
 parameter. It instantiates a new std.stream.File (without the passed
 file name and closes it when opened within the destructor. The last part
 is where things are getting unclear for me. On the "file.isOpen()" call
 in the destructor a segmentation fault occurs. What is the problem with
 that?

Likely, it is this[1]: "[T]he order in which the garbage collector calls destructors for unreference objects is not specified. This means that when the garbage collector calls a destructor for an object of a class that has members that are references to garbage collected objects, those references may no longer be valid. This means that destructors cannot reference sub objects." [1]: http://digitalmars.com/d/2.0/class.html#destructors

That's it indeed, but I'll add that it's the File struct that is at fault. See this bug: <http://d.puremagic.com/issues/show_bug.cgi?id=4624> To make it short: when the File struct is located somewhere in the GC heap (like inside a class), File's destructor is buggy when a file is open. The only workaround I can think of is to call close() or detach() on the file struct before the garbage collectors kicks in (doing it in the destructor of your class is already too late, it must be done before the destructor gets called). In fact, it's generally a good idea to not wait for the GC to collect your objects before closing files, because waiting for the GC could take an indeterminate amount of time and leave files open for a long time (the GC only runs when needed). -- Michel Fortin michel.fortin michelf.com http://michelf.com/
Sep 26 2010
prev sibling next sibling parent Tom Kazimiers <2voodoo gmx.de> writes:
Hi Jérôme,

On 09/26/2010 10:59 PM, "Jérôme M. Berger" wrote:
 	The way I see it, there are two possible situations:
 
 1. You really need precise control as to when the file is closed. In
 that case, your class contains an explicit cleanup function that you
 call before dropping the last reference and you can close the file
 at that time;
 
 2. You only need to be sure that the file gets closed at some point
 but it doesn't really signify when. In that case, you let the GC
 collect your class and you don't close the file. Eventually the GC
 will collect the std.stream.File instance which will result in
 calling its destructor which will close the file without your
 needing to do anything about it.

That are probably the two choices I have, yes. I already have such a cleanup function and will probably go for that. Thanks, Tom
Sep 28 2010
prev sibling parent Tom Kazimiers <2voodoo gmx.de> writes:
Hi,

On 09/27/2010 02:09 PM, Steven Schveighoffer wrote:
 On Sun, 26 Sep 2010 20:20:18 -0400, Michel Fortin
 In fact, it's generally a good idea to not wait for the GC to collect
 your objects before closing files, because waiting for the GC could
 take an indeterminate amount of time and leave files open for a long
 time (the GC only runs when needed).

Yes, this is true no matter what File construct you use.

I'll keep that in mind and will go for a cleanup method that I have to call explicitely. Especially files should probably only be opened if in use or if one wants to prevent others from using it. Cheers, Tom
Sep 28 2010
prev sibling next sibling parent Tom Kazimiers <2voodoo gmx.de> writes:
Hi Simen,

On 09/26/2010 04:06 PM, Simen kjaeraas wrote:
 Likely, it is this[1]:
 
 "[T]he order in which the garbage collector calls destructors for
 unreference objects is not specified. This means that when the garbage
 collector calls a destructor for an object of a class that has members
 that are references to garbage collected objects, those references may
 no longer be valid. This means that destructors cannot reference sub
 objects."
 
 [1]: http://digitalmars.com/d/2.0/class.html#destructors

thanks for your reply. I did not know that the garbage collector works that way. Is the reason for that flexibility for the GC? What if I want to handle some destruction parts to sub-objects or if I want conditions while destruction, based on sub-objects? Or maybe I should generally avoid situations like that. Do you have any suggestion how I should make sure that the file gets closed on destruction? Cheers, Tom
Sep 26 2010
prev sibling next sibling parent "Simen kjaeraas" <simen.kjaras gmail.com> writes:
Tom Kazimiers <2voodoo gmx.de> wrote:

 Hi,

 On 09/26/2010 07:13 PM, bearophile wrote:
 Tom Kazimiers:

 Do you have any suggestion how I should make
 sure that the file gets closed on destruction?

Can you use scope(exit) or the std.stdio.File?

Well, I have no idea. You mentioning scope(exit) was actually the first time I heard of it. Unfortunately I have not found any resource about it. Do you have a link to point me in the right direction?

http://digitalmars.com/d/2.0/statement.html#ScopeGuardStatement -- Simen
Sep 26 2010
prev sibling next sibling parent "Lars T. Kyllingstad" <public kyllingen.NOSPAMnet> writes:
On Sun, 26 Sep 2010 20:55:33 +0200, Tom Kazimiers wrote:
 If I would use std.stdio.File, what would be different?

Well, for one thing you won't have to write your code all over again when std.stream is deprecated, which will happen soon. std.stdio.File is really what you should use for file I/O in new code. That said, there's a chance it does exactly what you want. You don't have to open a file on construction, there's an open() function which opens a file and assigns it to the File handle. Nor do you have to worry about closing the file in the destructor, as it is automatically closed the moment the last reference to it goes out of scope. Here are a few examples of how to use it: File file; // File's a struct, so no need to use 'new' // Read a text file line by line file.open("foo.txt"); foreach (line; file.byLine()) writeln(line); // Read a binary file in 4MB chunks file.open("foo.dat"); foreach (ubyte[] chunk; file.byChunk(4*1024)) doStuffWith(chunk); // Read up to 100 ints from a file file.open("myInts"); auto buffer = new int[100]; auto data = file.rawRead(buffer); -Lars
Sep 27 2010
prev sibling next sibling parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Sun, 26 Sep 2010 20:20:18 -0400, Michel Fortin  
<michel.fortin michelf.com> wrote:

 On 2010-09-26 10:06:33 -0400, "Simen kjaeraas" <simen.kjaras gmail.com>  
 said:

 Tom Kazimiers <2voodoo gmx.de> wrote:

 Hi,
  a file reading class of mine can be constructed with a filename as
 parameter. It instantiates a new std.stream.File (without the passed
 file name and closes it when opened within the destructor. The last  
 part
 is where things are getting unclear for me. On the "file.isOpen()" call
 in the destructor a segmentation fault occurs. What is the problem with
 that?

"[T]he order in which the garbage collector calls destructors for unreference objects is not specified. This means that when the garbage collector calls a destructor for an object of a class that has members that are references to garbage collected objects, those references may no longer be valid. This means that destructors cannot reference sub objects." [1]: http://digitalmars.com/d/2.0/class.html#destructors

That's it indeed, but I'll add that it's the File struct that is at fault. See this bug: <http://d.puremagic.com/issues/show_bug.cgi?id=4624>

He's using std.stream.File, not std.stdio.File.
 In fact, it's generally a good idea to not wait for the GC to collect  
 your objects before closing files, because waiting for the GC could take  
 an indeterminate amount of time and leave files open for a long time  
 (the GC only runs when needed).

Yes, this is true no matter what File construct you use. -Steve
Sep 27 2010
prev sibling parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Sun, 26 Sep 2010 11:17:07 -0400, Tom Kazimiers <2voodoo gmx.de> wrote:

 Hi Simen,

 On 09/26/2010 04:06 PM, Simen kjaeraas wrote:
 Likely, it is this[1]:

 "[T]he order in which the garbage collector calls destructors for
 unreference objects is not specified. This means that when the garbage
 collector calls a destructor for an object of a class that has members
 that are references to garbage collected objects, those references may
 no longer be valid. This means that destructors cannot reference sub
 objects."

 [1]: http://digitalmars.com/d/2.0/class.html#destructors

thanks for your reply. I did not know that the garbage collector works that way. Is the reason for that flexibility for the GC? What if I want to handle some destruction parts to sub-objects or if I want conditions while destruction, based on sub-objects?

Then do not use a destructor. There is no way currently for the destructor to know whether it's being called from the GC or not, so it is only safe to destroy resources *not* allocated by the GC. In fact, the only reason destructors are supported in classes is to destroy non-GC resources.
 Or maybe I should generally
 avoid situations like that. Do you have any suggestion how I should make
 sure that the file gets closed on destruction?

You don't have to worry about it -- let the GC do its job. If you think about it, your class instance may have the only reference to the File object. Since your class is being destroyed by the GC, it makes sense that the File object has no references to it, so it too is being destroyed by the GC. If you want sooner destruction, you should manually close it earlier via another function call (like close() or detach()). However, I will mention something that others missed, unlike in C++, the default protection in a class is public, so your public: attribute doesn't do much, and your file member is publicly accessible. -Steve
Sep 27 2010