[Tarantool-patches] [PATCH 01/16] fio: don't use shared buffer in pread()
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sat Mar 20 03:42:31 MSK 2021
fio:pread() used buffer.IBUF_SHARED, which might be reused after a
yield. As a result, if pread() was called from 2 different fibers
or in parallel with something else using IBUF_SHARED, it would
turn the buffer into garbage for all parallel usages.
The same problem existed for read(), and was fixed in
c7c24f841322528c17714482d150b769d0afcddb ("fio: Fix race condition
in fio.read"). But apparently pread() was missed.
What is worse, the original commit's test passed even without the
fix from that commit. Because it didn't check the results of
read()s called from 2 fibers.
The patch fixes pread() and adds a test covering both read() and
pread(). The old test from the original commit is dropped.
Follow up #3187
---
src/lua/fio.lua | 3 +-
test/app/fio.result | 160 ++++++++++++++++++++++++------------------
test/app/fio.test.lua | 67 ++++++++++--------
3 files changed, 129 insertions(+), 101 deletions(-)
diff --git a/src/lua/fio.lua b/src/lua/fio.lua
index 954c44cdf..5db8bdea3 100644
--- a/src/lua/fio.lua
+++ b/src/lua/fio.lua
@@ -101,8 +101,7 @@ fio_methods.pread = function(self, buf, size, offset)
if not ffi.istype(const_char_ptr_t, buf) then
offset = size
size = buf
- tmpbuf = buffer.IBUF_SHARED
- tmpbuf:reset()
+ tmpbuf = buffer.ibuf()
buf = tmpbuf:reserve(size)
end
local res, err = internal.pread(self.fh, buf, size, offset)
diff --git a/test/app/fio.result b/test/app/fio.result
index 20cfe345d..8b5716658 100644
--- a/test/app/fio.result
+++ b/test/app/fio.result
@@ -1380,76 +1380,6 @@ fio.unlink(tmpfile)
---
- true
...
-tmp1 = fio.pathjoin(tmpdir, "tmp1")
----
-...
-tmp2= fio.pathjoin(tmpdir, "tmp2")
----
-...
-test_run:cmd("setopt delimiter ';'")
----
-- true
-...
-function write_file(name, odd)
- local fh = fio.open(name, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, tonumber('0777', 8))
- if odd then
- fh:write(string.rep('1', 100))
- else
- fh:write(string.rep('2', 100))
- end
- fh:write(name)
- fh:seek(0)
- return fh
-end;
----
-...
-test_run:cmd("setopt delimiter ''");
----
-- true
-...
-fh1 = write_file(tmp1)
----
-...
-fh2 = write_file(tmp2)
----
-...
-fiber = require('fiber')
----
-...
-digest = require('digest')
----
-...
-str = fh1:read()
----
-...
-fh1:seek(0)
----
-- 0
-...
-hash = digest.crc32(str)
----
-...
-ch = fiber.channel(1)
----
-...
-f1 = fiber.create(function() str = fh1:read() ch:put(digest.crc32(str)) end)
----
-...
-f2 = fiber.create(function() str = fh2:read() end)
----
-...
-ch:get() == hash
----
-- true
-...
-fio.unlink(tmp1)
----
-- true
-...
-fio.unlink(tmp2)
----
-- true
-...
fio.rmdir(tmpdir)
---
- true
@@ -1602,3 +1532,93 @@ tmpdir = nil
os.setenv('TMPDIR', old_tmpdir)
---
...
+--
+-- read() and pread() should not use a shared buffer so as not to clash with
+-- other fibers during yield.
+--
+rights = tonumber('0777', 8)
+---
+...
+flags = {'O_RDWR', 'O_TRUNC', 'O_CREAT'}
+---
+...
+tmpdir = fio.tempdir()
+---
+...
+file1 = fio.pathjoin(tmpdir, 'file1')
+---
+...
+file2 = fio.pathjoin(tmpdir, 'file2')
+---
+...
+fd1 = fio.open(file1, flags, rights)
+---
+...
+fd2 = fio.open(file2, flags, rights)
+---
+...
+fd1:write('1'), fd1:seek(0)
+---
+- true
+- 0
+...
+fd2:write('2'), fd2:seek(0)
+---
+- true
+- 0
+...
+res1, res2 = nil
+---
+...
+do \
+ fiber.new(function() res1 = fd1:read() end) \
+ fiber.new(function() res2 = fd2:read() end) \
+end
+---
+...
+_ = test_run:wait_cond(function() return res1 and res2 end)
+---
+...
+assert(res1 == '1')
+---
+- true
+...
+assert(res2 == '2')
+---
+- true
+...
+--
+-- The same with pread().
+--
+res1, res2 = nil
+---
+...
+do \
+ fiber.new(function() res1 = fd1:pread(1, 0) end) \
+ fiber.new(function() res2 = fd2:pread(1, 0) end) \
+end
+---
+...
+_ = test_run:wait_cond(function() return res1 and res2 end)
+---
+...
+assert(res1 == '1')
+---
+- true
+...
+assert(res2 == '2')
+---
+- true
+...
+fd1:close()
+---
+- true
+...
+fd2:close()
+---
+- true
+...
+fio.rmtree(tmpdir)
+---
+- true
+...
diff --git a/test/app/fio.test.lua b/test/app/fio.test.lua
index d1a20e8e5..085d72110 100644
--- a/test/app/fio.test.lua
+++ b/test/app/fio.test.lua
@@ -442,35 +442,6 @@ fio.path.lexists(link)
fio.unlink(link)
fio.unlink(tmpfile)
-tmp1 = fio.pathjoin(tmpdir, "tmp1")
-tmp2= fio.pathjoin(tmpdir, "tmp2")
-test_run:cmd("setopt delimiter ';'")
-function write_file(name, odd)
- local fh = fio.open(name, { 'O_RDWR', 'O_TRUNC', 'O_CREAT' }, tonumber('0777', 8))
- if odd then
- fh:write(string.rep('1', 100))
- else
- fh:write(string.rep('2', 100))
- end
- fh:write(name)
- fh:seek(0)
- return fh
-end;
-test_run:cmd("setopt delimiter ''");
-fh1 = write_file(tmp1)
-fh2 = write_file(tmp2)
-fiber = require('fiber')
-digest = require('digest')
-str = fh1:read()
-fh1:seek(0)
-hash = digest.crc32(str)
-ch = fiber.channel(1)
-f1 = fiber.create(function() str = fh1:read() ch:put(digest.crc32(str)) end)
-f2 = fiber.create(function() str = fh2:read() end)
-ch:get() == hash
-
-fio.unlink(tmp1)
-fio.unlink(tmp2)
fio.rmdir(tmpdir)
--
@@ -526,3 +497,41 @@ fio.tempdir()
tmpdir = nil
os.setenv('TMPDIR', old_tmpdir)
+
+--
+-- read() and pread() should not use a shared buffer so as not to clash with
+-- other fibers during yield.
+--
+rights = tonumber('0777', 8)
+flags = {'O_RDWR', 'O_TRUNC', 'O_CREAT'}
+tmpdir = fio.tempdir()
+file1 = fio.pathjoin(tmpdir, 'file1')
+file2 = fio.pathjoin(tmpdir, 'file2')
+fd1 = fio.open(file1, flags, rights)
+fd2 = fio.open(file2, flags, rights)
+fd1:write('1'), fd1:seek(0)
+fd2:write('2'), fd2:seek(0)
+
+res1, res2 = nil
+do \
+ fiber.new(function() res1 = fd1:read() end) \
+ fiber.new(function() res2 = fd2:read() end) \
+end
+_ = test_run:wait_cond(function() return res1 and res2 end)
+assert(res1 == '1')
+assert(res2 == '2')
+--
+-- The same with pread().
+--
+res1, res2 = nil
+do \
+ fiber.new(function() res1 = fd1:pread(1, 0) end) \
+ fiber.new(function() res2 = fd2:pread(1, 0) end) \
+end
+_ = test_run:wait_cond(function() return res1 and res2 end)
+assert(res1 == '1')
+assert(res2 == '2')
+
+fd1:close()
+fd2:close()
+fio.rmtree(tmpdir)
--
2.24.3 (Apple Git-128)
More information about the Tarantool-patches
mailing list