Mark non-private mapped files as external.

ioquatix · ioquatix · commit c86bcd434da5 · 2021-12-21T12:25:42.000+13:00
diff --git a/include/ruby/io/buffer.h b/include/ruby/io/buffer.h
@@ -27,20 +27,24 @@ RUBY_EXTERN size_t RUBY_IO_BUFFER_DEFAULT_SIZE;
 
 enum rb_io_buffer_flags {
     // The memory in the buffer is owned by someone else.
-    RB_IO_BUFFER_EXTERNAL = 0,
+    // More specifically, it means that someone else owns the buffer and we shouldn't try to resize it.
+    RB_IO_BUFFER_EXTERNAL = 1,
     // The memory in the buffer is allocated internally.
-    RB_IO_BUFFER_INTERNAL = 1,
+    RB_IO_BUFFER_INTERNAL = 2,
     // The memory in the buffer is mapped.
-    RB_IO_BUFFER_MAPPED = 2,
+    // A non-private mapping is marked as external.
+    RB_IO_BUFFER_MAPPED = 4,
 
     // The buffer is locked and cannot be resized.
-    RB_IO_BUFFER_LOCKED = 16,
+    // More specifically, it means we can't change the base address or size.
+    // A buffer is typically locked before a system call that uses the data.
+    RB_IO_BUFFER_LOCKED = 32,
 
     // The buffer mapping is private and will not impact other processes or the underlying file.
-    RB_IO_BUFFER_PRIVATE = 32,
+    RB_IO_BUFFER_PRIVATE = 64,
 
     // The buffer is read-only and cannot be modified.
-    RB_IO_BUFFER_IMMUTABLE = 64
+    RB_IO_BUFFER_IMMUTABLE = 128
 };
 
 enum rb_io_buffer_endian {
diff --git a/io_buffer.c b/io_buffer.c
@@ -83,6 +83,9 @@ io_buffer_map_file(struct rb_io_buffer *data, int descriptor, size_t size, off_t
     if (flags & RB_IO_BUFFER_PRIVATE) {
         access |= FILE_MAP_COPY;
         data->flags |= RB_IO_BUFFER_PRIVATE;
+    } else {
+        // This buffer refers to external data.
+        data->flags |= RB_IO_BUFFER_EXTERNAL;
     }
 
     void *base = MapViewOfFile(mapping, access, (DWORD)(offset >> 32), (DWORD)(offset & 0xFFFFFFFF), size);
@@ -107,6 +110,8 @@ io_buffer_map_file(struct rb_io_buffer *data, int descriptor, size_t size, off_t
         data->flags |= RB_IO_BUFFER_PRIVATE;
     }
     else {
+        // This buffer refers to external data.
+        data->flags |= RB_IO_BUFFER_EXTERNAL;
         access |= MAP_SHARED;
     }
 
@@ -572,7 +577,7 @@ rb_io_buffer_external_p(VALUE self)
     struct rb_io_buffer *data = NULL;
     TypedData_Get_Struct(self, struct rb_io_buffer, &rb_io_buffer_type, data);
 
-    return data->flags & (RB_IO_BUFFER_INTERNAL | RB_IO_BUFFER_MAPPED) ? Qfalse : Qtrue;
+    return data->flags & RB_IO_BUFFER_EXTERNAL ? Qtrue : Qfalse;
 }
 
 static VALUE
@@ -611,12 +616,6 @@ rb_io_buffer_immutable_p(VALUE self)
     return data->flags & RB_IO_BUFFER_IMMUTABLE ? Qtrue : Qfalse;
 }
 
-static int
-io_buffer_external_p(enum rb_io_buffer_flags flags)
-{
-    return !(flags & (RB_IO_BUFFER_INTERNAL | RB_IO_BUFFER_MAPPED));
-}
-
 VALUE
 rb_io_buffer_lock(VALUE self)
 {
@@ -858,7 +857,7 @@ rb_io_buffer_resize(VALUE self, size_t size)
         return;
     }
 
-    if (io_buffer_external_p(data->flags)) {
+    if (data->flags & RB_IO_BUFFER_EXTERNAL) {
         rb_raise(rb_eIOBufferMutationError, "Cannot resize external buffer!");
     }
 
diff --git a/test/ruby/test_io_buffer.rb b/test/ruby/test_io_buffer.rb
@@ -18,14 +18,14 @@ def assert_positive(value)
   end
 
   def test_flags
-    assert_equal 0, IO::Buffer::EXTERNAL
-    assert_equal 1, IO::Buffer::INTERNAL
-    assert_equal 2, IO::Buffer::MAPPED
+    assert_equal 1, IO::Buffer::EXTERNAL
+    assert_equal 2, IO::Buffer::INTERNAL
+    assert_equal 4, IO::Buffer::MAPPED
 
-    assert_equal 16, IO::Buffer::LOCKED
-    assert_equal 32, IO::Buffer::PRIVATE
+    assert_equal 32, IO::Buffer::LOCKED
+    assert_equal 64, IO::Buffer::PRIVATE
 
-    assert_equal 64, IO::Buffer::IMMUTABLE
+    assert_equal 128, IO::Buffer::IMMUTABLE
   end
 
   def test_endian

Original file line number	Diff line number	Diff line change
`@@ -83,6 +83,9 @@ io_buffer_map_file(struct rb_io_buffer *data, int descriptor, size_t size, off_t`
`83`	`83`	`if (flags & RB_IO_BUFFER_PRIVATE) {`
`84`	`84`	`access \|= FILE_MAP_COPY;`
`85`	`85`	`data->flags \|= RB_IO_BUFFER_PRIVATE;`
	`86`	`+ } else {`
	`87`	`+ // This buffer refers to external data.`
	`88`	`+ data->flags \|= RB_IO_BUFFER_EXTERNAL;`
`86`	`89`	`}`
`87`	`90`
`88`	`91`	`void *base = MapViewOfFile(mapping, access, (DWORD)(offset >> 32), (DWORD)(offset & 0xFFFFFFFF), size);`
`@@ -107,6 +110,8 @@ io_buffer_map_file(struct rb_io_buffer *data, int descriptor, size_t size, off_t`
`107`	`110`	`data->flags \|= RB_IO_BUFFER_PRIVATE;`
`108`	`111`	`}`
`109`	`112`	`else {`
	`113`	`+ // This buffer refers to external data.`
	`114`	`+ data->flags \|= RB_IO_BUFFER_EXTERNAL;`
`110`	`115`	`access \|= MAP_SHARED;`
`111`	`116`	`}`
`112`	`117`
`@@ -572,7 +577,7 @@ rb_io_buffer_external_p(VALUE self)`
`572`	`577`	`struct rb_io_buffer *data = NULL;`
`573`	`578`	`TypedData_Get_Struct(self, struct rb_io_buffer, &rb_io_buffer_type, data);`
`574`	`579`
`575`		`- return data->flags & (RB_IO_BUFFER_INTERNAL \| RB_IO_BUFFER_MAPPED) ? Qfalse : Qtrue;`
	`580`	`+ return data->flags & RB_IO_BUFFER_EXTERNAL ? Qtrue : Qfalse;`
`576`	`581`	`}`
`577`	`582`
`578`	`583`	`static VALUE`
`@@ -611,12 +616,6 @@ rb_io_buffer_immutable_p(VALUE self)`
`611`	`616`	`return data->flags & RB_IO_BUFFER_IMMUTABLE ? Qtrue : Qfalse;`
`612`	`617`	`}`
`613`	`618`
`614`		`-static int`
`615`		`-io_buffer_external_p(enum rb_io_buffer_flags flags)`
`616`		`-{`
`617`		`- return !(flags & (RB_IO_BUFFER_INTERNAL \| RB_IO_BUFFER_MAPPED));`
`618`		`-}`
`619`		`-`
`620`	`619`	`VALUE`
`621`	`620`	`rb_io_buffer_lock(VALUE self)`
`622`	`621`	`{`
`@@ -858,7 +857,7 @@ rb_io_buffer_resize(VALUE self, size_t size)`
`858`	`857`	`return;`
`859`	`858`	`}`
`860`	`859`
`861`		`- if (io_buffer_external_p(data->flags)) {`
	`860`	`+ if (data->flags & RB_IO_BUFFER_EXTERNAL) {`
`862`	`861`	`rb_raise(rb_eIOBufferMutationError, "Cannot resize external buffer!");`
`863`	`862`	`}`
`864`	`863`