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 C310E6EC40; Mon, 5 Jul 2021 09:31:37 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C310E6EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1625466697; bh=JpXu8KwiOWks+npPSFyjITXMPP1iGECiNBbjrf8T08Y=; 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=lLtOZ378G6IOIa1ZwUodCiD9seKWQveY6cbEuyDvF3kPvkmZvRfjzfTgTtDAyvIQi 5zLTqAAWt9GanortbPUvNxdB/sXh3rk7oYMyW0xARoEZqsTMPy3tfyWJvP6W/CxyaH EcYcBHrLBGUNXVRgB2OOvg+MpAvmTgXA+E/cOM2A= Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 502C76EC40 for ; Mon, 5 Jul 2021 09:31:16 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 502C76EC40 Received: by smtp16.mail.ru with esmtpa (envelope-from ) id 1m0I8Q-0008WU-N5; Mon, 05 Jul 2021 09:31:15 +0300 Date: Mon, 5 Jul 2021 09:30:47 +0300 To: Vladislav Shpilevoy Message-ID: <20210705063047.uwg6aby2dg44wqbm@tkn_work_nb> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD954DFF1DC42D673FB703477AD6D36A6E3C7EF2853D6A1C7C1182A05F538085040AF69FC4D3C9D14A29AB156429642329E093F42C4026AEBDC3438A10E7BABEB47 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7965AF5021CACFC74C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE7F8E53417176C7207EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BBCA57AF85F7723F2DF49C9E04003530DAEDA0A6224EDF8D9CC7F00164DA146DAFE8445B8C89999728AA50765F790063783E00425F71A4181389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8A9FF340AA05FB58CF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C98E93883770458356E0066C2D8992A164AD6D5ED66289B52698AB9A7B718F8C46E0066C2D8992A16725E5C173C3A84C33E1E9CDD035F181ABA3038C0950A5D36B5C8C57E37DE458B0BC6067A898B09E46D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7588D3C263EAE74EA731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A5012CF8F9AD05D4B388EE3D25F54F07527D27F2EE60D4FB1DD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7569E77FCA7B33833F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3450C5E6D685282BA1903969D464995067D5A12286DF95DA13FD32320810246D276B324A1DA55D85E51D7E09C32AA3244C458A3CC2E1DA14A97B914B542348D2F405AB220A9D022EBC927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj5fH2RN9TpJmJwV4Bp8vWNA== X-Mailru-Sender: FFAA8E4AEE17E37C3731A083A1A85ADEB6FC7DC86987906030B5BC18B1D9C615B7EA9FE7735C3DBFC664A44C781FCEA7C77752E0C033A69EDF9F2CE1E9CF805D8CD356D4F938FF726C18EFA0BB12DBB0 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH 0/4] RFC: Isolate serializer helpers 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: Alexander Turenko via Tarantool-patches Reply-To: Alexander Turenko Cc: Cyrill Gorcunov , tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" On Sun, Jul 04, 2021 at 03:09:07PM +0200, Vladislav Shpilevoy wrote: > Hi! Thanks for the patchset! > > On 23.06.2021 21:12, Alexander Turenko via Tarantool-patches wrote: > > Moved the serializer helpers into its own compilation unit, add some > > comments and a basic test: everything is just to simplify diving into > > this code. > > > > Guys, please, look, whether it seems useful enough to include into > > tarantool's mainline? Should we name it serializer.[ch] or > > somehow like serializer_helpers.[ch]? > > > > Part of https://github.com/tarantool/tarantool/issues/3228 > > Are you sure you need to fix it? It looks like a regular leg shooting. > It might be simple to detect in the case described by Mons, but what if > the recursion is not so easily visible? > > setmetatable({},{ > __serialize = function(a) > return {{{{a}}}} > end > }) > > You would need to use recursion detection algorithms like the one > we used to ask on interviews. And I am not sure it is worth it if > it can't be done in a simple way. I think it worth to rearrange the code and add a test disregarding whether we'll decide to fix or leave the problem. I'll update the issue on the week with description of all problems found around __serialize (see at end of the email as well). After this I'll ask Roman to update its patch (I'll add a checklist what should be done and how). I'll keep you in CC for those discussions. So you'll have ability to say 'it looks to complex' at any stage. In my opinion, it is highly undesirable to get segfault (or even a Lua error) from a serializer, because it is often used for logging. More or less correct result is better than fail. Even if the passed Lua object is ill-formed in some way. (However, sure, I want to keep the code as readable as possible and I would not accept a solution that is hard for me to dive into. I hope we'll implement something well balanced.) To be honest, even our usual "unsupported Lua type 'function'" error (which is raised for a function if `encode_use_tostring` is not `true`) is often undesirable. Raw idea: provide a helper like `yaml.encode_noxc()`, which will never raise an error and will be suitable for logging in the general case (it'll set `encode_use_tostring` under the hood). WBR, Alexander Turenko. ---- Sure, there are two problems with __serialize, which lead to segfault: - recursion within single Lua object serialization; - recursion over several Lua objects. But there is one problem of another kind. A return value of __serialize does not participate in references search. | local x = {whoami = 'x'} | yaml.encode({ | foo = x, | bar = setmetatable({}, {__serialize = function(_) return x end}) | }) ** now ** | --- | foo: | whoami: x | bar: | whoami: x | ... ** should be ** | --- | foo: &1 | whoami: x | bar: *1 | ...