Hi, Max
Thanks for the patch! LGTM
See my comments below.
Sergey
<snipped>
diff --git a/test/tarantool-tests/lj-946-print-errors-from-gc-fin-custom.test.lua b/test/tarantool-tests/lj-946-print-errors-from-gc-fin-custom.test.lua new file mode 100644 index 00000000..71efc260 --- /dev/null +++ b/test/tarantool-tests/lj-946-print-errors-from-gc-fin-custom.test.lua @@ -0,0 +1,42 @@ +local tap = require('tap') +local test = tap.test('lj-946-print-errors-from-gc-fin-custom'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(2) + +local ffi = require('ffi') +local error_in_finalizer = false + +local function errfin_handler() + error_in_finalizer = true +end + +local function new_bad_cdata() + return ffi.gc(ffi.new('char [?]', 1024), 'uncallable string') +end + +local function test_f() + collectgarbage('collect') + -- Make GC aggressive enough to end the atomic phase before + -- exiting the trace. + collectgarbage('setstepmul', 400) + -- The number of iterations is empirical, just big enough for the + -- issue to strike. + for _ = 1, 4000 do
Works fine with a number of iterations equal to 300,
checked locally by running test 1000 times.
+ new_bad_cdata() + end +end + +jit.opt.start('hotloop=1') +-- Handler is registered but never called before the patch. +-- It should be called after the patch. +jit.attach(errfin_handler, 'errfin') +local status = pcall(test_f) +-- We have to stop GC now because any step raises the error due to +-- cursed cdata objects. +collectgarbage('stop') +test:ok(status, 'test function completed successfully') +test:ok(error_in_finalizer, 'error handler called') + +test:done(true) diff --git a/test/tarantool-tests/lj-946-print-errors-from-gc-fin-default.test.lua b/test/tarantool-tests/lj-946-print-errors-from-gc-fin-default.test.lua new file mode 100644 index 00000000..dfef11e5 --- /dev/null +++ b/test/tarantool-tests/lj-946-print-errors-from-gc-fin-default.test.lua @@ -0,0 +1,11 @@ +local tap = require('tap') +local test = tap.test('lj-flush-on-trace'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(1) + +local script = require('utils').exec.makecmd(arg, { redirect = '2>&1' }) +local output = script() +test:like(output, '.*ERROR in finalizer:.*')
testcase description is missed:
TAP version 13
1..1
ok - nil
+test:done(true) diff --git a/test/tarantool-tests/lj-946-print-errors-from-gc-fin-default/script.lua b/test/tarantool-tests/lj-946-print-errors-from-gc-fin-default/script.lua new file mode 100644 index 00000000..fdd9ced1 --- /dev/null +++ b/test/tarantool-tests/lj-946-print-errors-from-gc-fin-default/script.lua @@ -0,0 +1,24 @@ +local ffi = require('ffi') + +local function new_bad_cdata() + return ffi.gc(ffi.new('char [?]', 1024), 'uncallable string') +end + +local function test_f() + collectgarbage('collect') + -- Make GC aggressive enough to end the atomic phase before + -- exiting the trace. + collectgarbage('setstepmul', 400) + -- The number of iterations is empirical, just big enough for the + -- issue to strike. + for _ = 1, 4000 do + new_bad_cdata() + end +end + +jit.opt.start('hotloop=1') +local status = pcall(test_f) +-- We have to stop GC now because any step raises the error due to +-- cursed cdata objects. +collectgarbage('stop') +assert(status, 'error is not rethrown') -- 2.39.3 (Apple Git-145)