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 DC08F6FC8F; Thu, 25 Mar 2021 00:25:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org DC08F6FC8F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1616621138; bh=trjxfbJ4Y2Skrgg6U1DBR9Gp/YR3tuiiutM3b7ehDA4=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=IXruvfWKkFZM3RJErQGONJ5nJz/w9i4GFkqLHMhBxAc7NFOOVY2le4wNCG8ZqG9r/ nbh8IlOvIv9/AIR+1DingSnf2Pq36Sv/JPA2tTaEyeut/tVec77hEwEVaqUp56IzG7 CdMqbcHNB56mzIlRDrTGPaVZQbR5Sk+uA0jI5e4g= Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (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 5298C6BD36 for ; Thu, 25 Mar 2021 00:24:39 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5298C6BD36 Received: by smtp48.i.mail.ru with esmtpa (envelope-from ) id 1lPAzW-0004ib-Ag; Thu, 25 Mar 2021 00:24:38 +0300 To: tarantool-patches@dev.tarantool.org, kyukhin@tarantool.org Date: Wed, 24 Mar 2021 22:24:22 +0100 Message-Id: <077274ab8c28a37a1524636fbfbbed56cb65c7ab.1616620860.git.v.shpilevoy@tarantool.org> X-Mailer: git-send-email 2.24.3 (Apple Git-128) In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD9064ADF4728AA0EE9C29AB0FA1CCF417B8CF9BD94A36D3F02182A05F53808504032DBD79EC37695B0216B7E54FC900F4B9806F17EECC0A88383259FB48D880C1B X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7832AD58179B12F4FEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006379703D7D5B138802D8638F802B75D45FF914D58D5BE9E6BC131B5C99E7648C95C5DD32608FC869F5D1290C563A6D3B78A75003894E329BD36A471835C12D1D9774AD6D5ED66289B5278DA827A17800CE759A2DA0C93DFCD719FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C3FB12F4B11BB5604F117882F4460429728AD0CFFFB425014E868A13BD56FB6657D81D268191BDAD3DC09775C1D3CA48CFBB339968D8EBD1C6BA3038C0950A5D36C8A9BA7A39EFB766EC990983EF5C0329BA3038C0950A5D36D5E8D9A59859A8B6CCE505037BFD814176E601842F6C81A1F004C906525384307823802FF610243DF43C7A68FF6260569E8FC8737B5C2249EC8D19AE6D49635B3BBE47FD9DD3FB59A8DF7F3B2552694A2BEBFE083D3B9BA73A03B725D353964BB11811A4A51E3B096D1867E19FE14079BA9C0B312567BB23089D37D7C0E48F6CA18204E546F3947C83C798A30B85E16BC6EABA9B74D0DA47C8A9BA7A39EFB7666BA297DBC24807EA089D37D7C0E48F6C8AA50765F7900637B8F435DEDE9E76EBEFF80C71ABB335746BA297DBC24807EA27F269C8F02392CD6143B1329D1CCA5627F269C8F02392CD5571747095F342E88FB05168BE4CE3AF X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A2368A440D3B0F6089093C9A16E5BC824AC8B6CDF511875BC4E8F7B195E1C97831148145A0506FFD4857D12FD7D753A721 X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975C5DD32608FC869F5D1290C563A6D3B78A75003894E329BD369C2B6934AE262D3EE7EAB7254005DCED7532B743992DF240BDC6A1CF3F042BAD6DF99611D93F60EF0417BEADF48D1460699F904B3F4130E343918A1A30D5E7FCCB5012B2E24CD356 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D341E08D7EE1804B76355F50DB0A23D2D4E5B034E630929778F3ACBBE6A44E43957EC5A4667BB1FB0DA1D7E09C32AA3244C2ECBB5539D6FA9DFF4FF037868EDB8CB7C0C08F7987826B9729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojjqzNotmU+gcb9QlsLUKtvg== X-Mailru-Sender: 504CC1E875BF3E7D9BC0E5172ADA3110A906815429F9D9606E06629DD435FEB9041D7EBBE128562B07784C02288277CA03E0582D3806FB6A5317862B1921BA260ED6CFD6382C13A6112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: [Tarantool-patches] [PATCH 10/15] uri: replace static_alloc with ffi stash and ibuf 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: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" static_alloc() appears not to be safe to use in Lua, because it does not provide any ownership protection for the returned values. The problem appears when something is allocated, then Lua GC starts, and some __gc handlers might also use static_alloc(). In Lua and in C - both lead to the buffer being corrupted in its original usage place. The patch is a part of activity of getting rid of static_alloc() in Lua. It removes it from uri Lua module and makes it use the new FFI stash feature, which helps to cache frequently used and heavy to allocate FFI values. In one place static_alloc() was used for an actual buffer - it was replaced with cord_ibuf which is equally fast when preallocated. ffi.new() for temporary struct uri is not used, because - It produces a new GC object; - ffi.new('struct uri') costs around 20ns while FFI stash costs around 0.8ns. The hack with 'struct uri[1]' does not help because size of uri is > 128 bytes; - Without JIT ffi.new() costs about the same as the stash, not better as well; The patch makes uri perf a bit better in the places where static_alloc() was used, because its cost was around 7ns for one allocation. (cherry picked from commit 7175b43e842fe42a04bba2b88006732f20bd7552) --- src/lua/uri.lua | 22 ++++++-- test/app-tap/gh-5632-gc-buf-reuse.test.lua | 60 +++++++++++++++++++++- 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/src/lua/uri.lua b/src/lua/uri.lua index d2946cd2d..98f4e02ec 100644 --- a/src/lua/uri.lua +++ b/src/lua/uri.lua @@ -1,6 +1,7 @@ -- uri.lua (internal file) local ffi = require('ffi') +local buffer = require('buffer') ffi.cdef[[ struct uri { @@ -31,14 +32,19 @@ uri_format(char *str, size_t len, struct uri *uri, bool write_password); ]] local builtin = ffi.C; - -local uribuf = ffi.new('struct uri') +local uri_stash = buffer.ffi_stash_new('struct uri') +local uri_stash_take = uri_stash.take +local uri_stash_put = uri_stash.put +local cord_ibuf_take = buffer.internal.cord_ibuf_take +local cord_ibuf_put = buffer.internal.cord_ibuf_put local function parse(str) if str == nil then error("Usage: uri.parse(string)") end + local uribuf = uri_stash_take() if builtin.uri_parse(uribuf, str) ~= 0 then + uri_stash_put(uribuf) return nil end local result = {} @@ -55,10 +61,12 @@ local function parse(str) elseif uribuf.host_hint == 3 then result.unix = result.service end + uri_stash_put(uribuf) return result end local function format(uri, write_password) + local uribuf = uri_stash_take() uribuf.scheme = uri.scheme uribuf.scheme_len = string.len(uri.scheme or '') uribuf.login = uri.login @@ -75,9 +83,13 @@ local function format(uri, write_password) uribuf.query_len = string.len(uri.query or '') uribuf.fragment = uri.fragment uribuf.fragment_len = string.len(uri.fragment or '') - local str = ffi.new('char[1024]') - builtin.uri_format(str, 1024, uribuf, write_password and 1 or 0) - return ffi.string(str) + local ibuf = cord_ibuf_take() + local str = ibuf:alloc(1024) + local len = builtin.uri_format(str, 1024, uribuf, write_password and 1 or 0) + uri_stash_put(uribuf) + str = ffi.string(str, len) + cord_ibuf_put(ibuf) + return str end return { diff --git a/test/app-tap/gh-5632-gc-buf-reuse.test.lua b/test/app-tap/gh-5632-gc-buf-reuse.test.lua index b09b1bf3e..81dafd36e 100755 --- a/test/app-tap/gh-5632-gc-buf-reuse.test.lua +++ b/test/app-tap/gh-5632-gc-buf-reuse.test.lua @@ -10,6 +10,7 @@ local tap = require('tap') local ffi = require('ffi') local uuid = require('uuid') +local uri = require('uri') local function test_uuid(test) test:plan(1) @@ -42,8 +43,65 @@ local function test_uuid(test) test:ok(is_success, 'uuid in gc') end +local function test_uri(test) + test:plan(1) + + local gc_count = 100 + local iter_count = 1000 + local port = 1 + local ip = 1 + local login = 1 + local pass = 1 + local is_success = true + + local function uri_parse() + local loc_ip = ip + local loc_port = port + local loc_pass = pass + local loc_login = login + + ip = ip + 1 + port = port + 1 + pass = pass + 1 + login = login + 1 + if port > 60000 then + port = 1 + end + if ip > 255 then + ip = 1 + end + + loc_ip = string.format('127.0.0.%s', loc_ip) + loc_port = tostring(loc_port) + loc_pass = string.format('password%s', loc_pass) + loc_login = string.format('login%s', loc_login) + local host = string.format('%s:%s@%s:%s', loc_login, loc_pass, + loc_ip, loc_port) + local u = uri.parse(host) + if u.host ~= loc_ip or u.login ~= loc_login or u.service ~= loc_port or + u.password ~= loc_pass then + is_success = false + assert(false) + end + end + + local function create_gc() + for _ = 1, gc_count do + ffi.gc(ffi.new('char[1]'), uri_parse) + end + end + + for _ = 1, iter_count do + create_gc() + uri_parse() + end + + test:ok(is_success, 'uri in gc') +end + local test = tap.test('gh-5632-gc-buf-reuse') -test:plan(1) +test:plan(2) test:test('uuid in __gc', test_uuid) +test:test('uri in __gc', test_uri) os.exit(test:check() and 0 or 1) -- 2.24.3 (Apple Git-128)