From 260bf60e52ffdfa625be1153624b0d123fc305f8 Mon Sep 17 00:00:00 2001
From: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Date: Mon, 25 Dec 2023 14:20:53 +1300
Subject: [PATCH] Correctly release the underlying file mapping. (#9340)

* Avoiding using `Tempfile` which was retaining the file preventing it from unlinking.
---
 include/ruby/io/buffer.h    |  3 +++
 io_buffer.c                 | 40 ++++++++++++++++++++-----------------
 test/ruby/test_io_buffer.rb | 15 +++++++-------
 3 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/include/ruby/io/buffer.h b/include/ruby/io/buffer.h
index 89efc0597d26a4..b044db053995bf 100644
--- a/include/ruby/io/buffer.h
+++ b/include/ruby/io/buffer.h
@@ -58,6 +58,9 @@ enum rb_io_buffer_flags {
 
     // The buffer is read-only and cannot be modified.
     RB_IO_BUFFER_READONLY = 128,
+
+    // The buffer is backed by a file.
+    RB_IO_BUFFER_FILE = 256,
 };
 
 // Represents the endian of the data types.
diff --git a/io_buffer.c b/io_buffer.c
index 49fa4e5d12d696..3c47f659bcab28 100644
--- a/io_buffer.c
+++ b/io_buffer.c
@@ -155,17 +155,7 @@ io_buffer_map_file(struct rb_io_buffer *buffer, int descriptor, size_t size, rb_
     buffer->size = size;
 
     buffer->flags |= RB_IO_BUFFER_MAPPED;
-}
-
-// Release the memory associated with a mapped buffer.
-static inline void
-io_buffer_unmap(void* base, size_t size)
-{
-#ifdef _WIN32
-    VirtualFree(base, 0, MEM_RELEASE);
-#else
-    munmap(base, size);
-#endif
+    buffer->flags |= RB_IO_BUFFER_FILE;
 }
 
 static void
@@ -234,7 +224,16 @@ io_buffer_free(struct rb_io_buffer *buffer)
         }
 
         if (buffer->flags & RB_IO_BUFFER_MAPPED) {
-            io_buffer_unmap(buffer->base, buffer->size);
+#ifdef _WIN32
+            if (buffer->flags & RB_IO_BUFFER_FILE) {
+                UnmapViewOfFile(buffer->base);
+            }
+            else {
+                VirtualFree(buffer->base, 0, MEM_RELEASE);
+            }
+#else
+            munmap(buffer->base, buffer->size);
+#endif
         }
 
         // Previously we had this, but we found out due to the way GC works, we
@@ -245,12 +244,6 @@ io_buffer_free(struct rb_io_buffer *buffer)
 
         buffer->base = NULL;
 
-#if defined(_WIN32)
-        if (buffer->mapping) {
-            CloseHandle(buffer->mapping);
-            buffer->mapping = NULL;
-        }
-#endif
         buffer->size = 0;
         buffer->flags = 0;
         buffer->source = Qnil;
@@ -258,6 +251,13 @@ io_buffer_free(struct rb_io_buffer *buffer)
         return 1;
     }
 
+#if defined(_WIN32)
+    if (buffer->mapping) {
+        CloseHandle(buffer->mapping);
+        buffer->mapping = NULL;
+    }
+#endif
+
     return 0;
 }
 
@@ -926,6 +926,10 @@ rb_io_buffer_to_s(VALUE self)
         rb_str_cat2(result, " MAPPED");
     }
 
+    if (buffer->flags & RB_IO_BUFFER_FILE) {
+        rb_str_cat2(result, " FILE");
+    }
+
     if (buffer->flags & RB_IO_BUFFER_SHARED) {
         rb_str_cat2(result, " SHARED");
     }
diff --git a/test/ruby/test_io_buffer.rb b/test/ruby/test_io_buffer.rb
index f7a6557a24df76..c4239101cfdaf9 100644
--- a/test/ruby/test_io_buffer.rb
+++ b/test/ruby/test_io_buffer.rb
@@ -519,23 +519,22 @@ def test_shared
   end
 
   def test_private
-    omit if RUBY_PLATFORM =~ /mswin|mingw/
+    tmpdir = Dir.tmpdir
+    buffer_path = File.join(tmpdir, "buffer.txt")
+    File.write(buffer_path, "Hello World")
 
-    Tempfile.create("buffer.txt") do |io|
-      io.write("Hello World")
-
-      buffer = IO::Buffer.map(io, nil, 0, IO::Buffer::PRIVATE)
+    File.open(buffer_path) do |file|
+      buffer = IO::Buffer.map(file, nil, 0, IO::Buffer::PRIVATE)
       assert buffer.private?
       refute buffer.readonly?
 
       buffer.set_string("J")
 
       # It was not changed because the mapping was private:
-      io.seek(0)
-      assert_equal "Hello World", io.read
+      file.seek(0)
+      assert_equal "Hello World", file.read
     ensure
       buffer&.free
-      io&.close
     end
   end
 end