Correctly release the underlying file mapping. (#9340)

* Avoiding using `Tempfile` which was retaining the file preventing it from unlinking.
This commit is contained in:
Samuel Williams 2023-12-25 14:20:53 +13:00 committed by GitHub
parent 5af64ff7db
commit 260bf60e52
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 32 additions and 26 deletions

View File

@ -58,6 +58,9 @@ enum rb_io_buffer_flags {
// The buffer is read-only and cannot be modified. // The buffer is read-only and cannot be modified.
RB_IO_BUFFER_READONLY = 128, RB_IO_BUFFER_READONLY = 128,
// The buffer is backed by a file.
RB_IO_BUFFER_FILE = 256,
}; };
// Represents the endian of the data types. // Represents the endian of the data types.

View File

@ -155,17 +155,7 @@ io_buffer_map_file(struct rb_io_buffer *buffer, int descriptor, size_t size, rb_
buffer->size = size; buffer->size = size;
buffer->flags |= RB_IO_BUFFER_MAPPED; buffer->flags |= RB_IO_BUFFER_MAPPED;
} buffer->flags |= RB_IO_BUFFER_FILE;
// 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
} }
static void static void
@ -234,7 +224,16 @@ io_buffer_free(struct rb_io_buffer *buffer)
} }
if (buffer->flags & RB_IO_BUFFER_MAPPED) { 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 // 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; buffer->base = NULL;
#if defined(_WIN32)
if (buffer->mapping) {
CloseHandle(buffer->mapping);
buffer->mapping = NULL;
}
#endif
buffer->size = 0; buffer->size = 0;
buffer->flags = 0; buffer->flags = 0;
buffer->source = Qnil; buffer->source = Qnil;
@ -258,6 +251,13 @@ io_buffer_free(struct rb_io_buffer *buffer)
return 1; return 1;
} }
#if defined(_WIN32)
if (buffer->mapping) {
CloseHandle(buffer->mapping);
buffer->mapping = NULL;
}
#endif
return 0; return 0;
} }
@ -926,6 +926,10 @@ rb_io_buffer_to_s(VALUE self)
rb_str_cat2(result, " MAPPED"); rb_str_cat2(result, " MAPPED");
} }
if (buffer->flags & RB_IO_BUFFER_FILE) {
rb_str_cat2(result, " FILE");
}
if (buffer->flags & RB_IO_BUFFER_SHARED) { if (buffer->flags & RB_IO_BUFFER_SHARED) {
rb_str_cat2(result, " SHARED"); rb_str_cat2(result, " SHARED");
} }

View File

@ -519,23 +519,22 @@ class TestIOBuffer < Test::Unit::TestCase
end end
def test_private 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| File.open(buffer_path) do |file|
io.write("Hello World") buffer = IO::Buffer.map(file, nil, 0, IO::Buffer::PRIVATE)
buffer = IO::Buffer.map(io, nil, 0, IO::Buffer::PRIVATE)
assert buffer.private? assert buffer.private?
refute buffer.readonly? refute buffer.readonly?
buffer.set_string("J") buffer.set_string("J")
# It was not changed because the mapping was private: # It was not changed because the mapping was private:
io.seek(0) file.seek(0)
assert_equal "Hello World", io.read assert_equal "Hello World", file.read
ensure ensure
buffer&.free buffer&.free
io&.close
end end
end end
end end