From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 3B934404481; Thu, 9 Nov 2023 15:14:53 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 3B934404481 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1699532093; bh=vhIJtWv92hQ8dqp6HskfO6/XFP7HGZp27TFIz/zHVvc=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=BXx9y5Qi9D7MazdfP0Si3RWDQHipgjTxyw6KFBXLSiBIBKxfQSTGSWj9jF+6mt2cG qL+2riG6tqdIlodVcNQrnyJZPy+6u2eSHjbdisak6bdgSf3mZ0uKbH2wF0Uitgdn/n Md3mPB57/2xbNYH3YgDw2k4zP8pLjQHuJftHSDQY= Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [95.163.41.89]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 91DB7404481 for ; Thu, 9 Nov 2023 15:14:51 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 91DB7404481 Received: by smtp54.i.mail.ru with esmtpa (envelope-from ) id 1r13vt-00FX31-1K; Thu, 09 Nov 2023 15:14:49 +0300 Date: Thu, 9 Nov 2023 15:14:49 +0300 To: Sergey Bronnikov Message-ID: References: <20231107210616.53138-1-max.kokryashkin@gmail.com> <20231107210616.53138-2-max.kokryashkin@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mailru-Src: smtp X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9C2A6B03AB739174C8B8ADBDCDAD54AB847BB7A66D7B1BE8200894C459B0CD1B9FFB296D9E72B0705E36836854A76E422D65D1FD401799EE8B4BCB0D8152A3B5C X-C1DE0DAB: 0D63561A33F958A535FF5AFA746844FF5EF94DE595041E2E3768244C0DDDEF18F87CCE6106E1FC07E67D4AC08A07B9B065B78C30F681404DCB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0AD5177F0B940C8B66ECE892A7B2722663E91682638B966EB3F662256BEEFA9527F0E432940A61DDD7F075061CF8E4C986B4C3E3C5E6742DECD4853E03755BD6FA455628FB2F137D7C237FD76D11AF80A0D635D2B8BACDD183C69F20C0A9DAF8B8FEA455F16B58544A21C197AAF4D2E4732965026E5D17F6739C77C69D99B9914278E50E1F0597A6FD5CD72808BE417F3B9E0E7457915DAA85F X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojKdAcnY08CHLYAY1+j9Ivuw== X-Mailru-Sender: 11C2EC085EDE56FA38FD4C59F7EFE40770FFB3E1EEA45A2D7E2E76672E94A59820D4C3049F133A78D51284F0FE6F529ABC7555A253F5B200DF104D74F62EE79D27EC13EC74F6107F4198E0F3ECE9B5443453F38A29522196 X-Mras: OK Subject: Re: [Tarantool-patches] [PATCH luajit v2 1/2] Print errors from __gc finalizers instead of rethrowing them. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Cc: Maksim Kokryashkin , tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the review! See my answers below. On Thu, Nov 09, 2023 at 03:03:46PM +0300, Sergey Bronnikov wrote: > Hi, Max > > > Thanks for the patch! LGTM > > See my comments below. > > Sergey > > On 11/8/23 00:06, Maksim Kokryashkin wrote: > > > > > > 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. Yep, works fine with smaller number of iterations under LuaJIT, but the story is different under Tarantool, that's why the number is so large. > > > + 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: Fixed. > > > 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) > > Here is the diff: === 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 index f0d11f6f..f22b3997 100644 --- 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 @@ -7,5 +7,5 @@ test:plan(1) local script = require('utils').exec.makecmd(arg, { redirect = '2>&1' }) local output = script() -test:like(output, 'ERROR in finalizer:') +test:like(output, 'ERROR in finalizer:', 'error handler called') test:done(true)