Merge pull request #3619 from CyberShadow/pull-20150901-074214

std.mmfile - fix multiple issues

std/mmfile.d

@@ -78,6 +78,8 @@ class MmFile
     version(linux) this(File file, Mode mode = Mode.read, ulong size = 0,
             void* address = null, size_t window = 0)
     {
+        // Save a copy of the File to make sure the fd stays open.
+        this.file = file;
         this(file.fileno, mode, size, address, window);
     }
 
@@ -234,13 +236,25 @@ class MmFile
             }
             else
                 hFile = INVALID_HANDLE_VALUE;
-            scope(failure) if (hFile != INVALID_HANDLE_VALUE) CloseHandle(hFile);
+
+            scope(failure)
+            {
+                if (hFile != INVALID_HANDLE_VALUE)
+                {
+                    CloseHandle(hFile);
+                    hFile = INVALID_HANDLE_VALUE;
+                }
+            }
 
             int hi = cast(int)(size>>32);
             hFileMap = CreateFileMappingW(hFile, null, flProtect,
                     hi, cast(uint)size, null);
             wenforce(hFileMap, "CreateFileMapping");
-            scope(failure) CloseHandle(hFileMap);
+            scope(failure)
+            {
+                CloseHandle(hFileMap);
+                hFileMap = null;
+            }
 
             if (size == 0 && filename != null)
             {
@@ -312,6 +326,7 @@ class MmFile
                 {
                     //printf("\tfstat error, errno = %d\n", errno);
                     .close(fd);
+                    fd = -1;
                     errnoEnforce(false, "Could not stat file "~filename);
                 }
 
@@ -335,10 +350,14 @@ class MmFile
             size_t initial_map = (window && 2*window<size)
                 ? 2*window : cast(size_t)size;
             p = mmap(address, initial_map, prot, flags, fd, 0);
-            if (p == MAP_FAILED) {
+            if (p == MAP_FAILED)
+            {
                 if (fd != -1)
+                {
                     .close(fd);
-                errnoEnforce(fd != -1, "Could not map file "~filename);
+                    fd = -1;
+                }
+                errnoEnforce(false, "Could not map file "~filename);
             }
 
             data = p[0 .. initial_map];
@@ -356,6 +375,7 @@ class MmFile
     {
         debug (MMFILE) printf("MmFile.~this()\n");
         unmap();
+        data = null;
         version (Windows)
         {
             wenforce(hFileMap == null || CloseHandle(hFileMap) == TRUE,
@@ -369,15 +389,24 @@ class MmFile
         }
         else version (Posix)
         {
-            // enforce shouldn't be here and close() is safe anyway
-            .close(fd);
+            version (linux)
+            {
+                if (file !is File.init)
+                {
+                    // The File destructor will close the file,
+                    // if it is the only remaining reference.
+                    return;
+                }
+            }
+            errnoEnforce(fd == -1 || fd <= 2
+                    || .close(fd) != -1,
+                    "Could not close handle");
             fd = -1;
         }
         else
         {
             static assert(0);
         }
-        data = null;
     }
 
     /* Flush any pending output.
@@ -549,6 +578,7 @@ private:
     ulong  size;
     Mode   mMode;
     void*  address;
+    version (linux) File file;
 
     version (Windows)
     {
@@ -623,3 +653,43 @@ unittest
     // Create anonymous mapping
     auto test = new MmFile(null, MmFile.Mode.readWriteNew, 1024*1024, null);
 }
+
+version(linux)
+unittest // Issue 14868
+{
+    import std.typecons : scoped;
+    import std.file : deleteme;
+
+    // Test retaining ownership of File/fd
+
+    auto fn = std.file.deleteme ~ "-testing.txt";
+    scope(exit) std.file.remove(fn);
+    File(fn, "wb").writeln("Testing!");
+    scoped!MmFile(File(fn));
+
+    // Test that unique ownership of File actually leads to the fd being closed
+
+    auto f = File(fn);
+    auto fd = f.fileno;
+    {
+        auto mf = scoped!MmFile(f);
+        f = File.init;
+    }
+    assert(.close(fd) == -1);
+}
+
+unittest // Issue 14994, 14995
+{
+    import std.file : deleteme;
+    import std.typecons : scoped;
+
+    // Zero-length map may or may not be valid on OSX
+    version (OSX)
+        import std.exception : verifyThrown = collectException;
+    else
+        import std.exception : verifyThrown = assertThrown;
+
+    auto fn = std.file.deleteme ~ "-testing.txt";
+    scope(exit) std.file.remove(fn);
+    verifyThrown(scoped!MmFile(fn, MmFile.Mode.readWrite, 0, null));
+}