www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 1250] New: std.stream.BufferedFile should have a destructor with close()

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

           Summary: std.stream.BufferedFile should have a destructor with
                    close()
           Product: D
           Version: 1.014
          Platform: PC
        OS/Version: Windows
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: bugzilla digitalmars.com
        ReportedBy: wbaxter gmail.com


std.stream.BufferedFile should have a destructor that calls close().
std.stream.File has one, but std.stream.BufferedFile does not.
It means not all the data will get written to disk with the usage pattern:
{
  scope f = new BufferedFile(filename,FileMode.Out);
  // do a bunch of writes
  // hope f gets closed by RAII magic (it won't)
}


--
May 28 2007
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1250


fvbommel wxs.nl changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         OS/Version|Windows                     |All
         Resolution|                            |INVALID




------- Comment #1 from fvbommel wxs.nl  2007-05-28 05:19 -------
You can't do that; it causes undefined behavior if BufferedFile is collected by
the GC. BufferedFile internally uses a File, and GC-collected objects can't
safely reference other GC-able objects in their destructors since they may have
been collected earlier and accessing deleted objects is Bad(TM).
File can call close() in the destructor because it doesn't need to access
GC-able objects to close(), it just performs the needed OS calls directly.

The only way to safely put a close() in the destructor of BufferedFile (or
BufferedStream, its superclass) would be to make it a "scope" class to require
deterministic destruction, but that would disallow a lot of use cases.

You can add a "scope(exit) f.close();" after the declaration of f to safely get
the closing behavior you want.


--
May 28 2007
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=1250





------- Comment #2 from wbaxter gmail.com  2007-05-28 13:48 -------
(In reply to comment #1)

 You can't do that; it causes undefined behavior if BufferedFile is collected by
 the GC. BufferedFile internally uses a File, and GC-collected objects can't
 safely reference other GC-able objects in their destructors since they may have
 been collected earlier and accessing deleted objects is Bad(TM).
 File can call close() in the destructor because it doesn't need to access
 GC-able objects to close(), it just performs the needed OS calls directly.


Ouch.  That kinda defeats one of the major purposes of having 'scope' in the
first place.  Sounds like D needs that "scope destruction" flag passed to the
destructor as has been suggested before, so that people have a way to make
their classes work optimally with scope.


 The only way to safely put a close() in the destructor of BufferedFile (or
 BufferedStream, its superclass) would be to make it a "scope" class to require
 deterministic destruction, but that would disallow a lot of use cases.


...which is basically always the case.  No one wants to slap a 'scope' on a
class because it just limits the possibilities.  I guess you could derive a
ScopeBufferedFile from BufferedFile and make that a scope class.

scope ScopeBufferedFile : BufferedFile {
   ~this() { m_file.close(); }
}

Seems kind of inelegant, though.


--
May 28 2007