From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 894F9452566 for ; Sat, 16 Nov 2019 15:18:58 +0300 (MSK) References: <0e1af790a8cbc380fcd3bb7c7295f5f5b80d98a3.1573862438.git.v.shpilevoy@tarantool.org> <20191116115421.GD14490@atlas> From: Vladislav Shpilevoy Message-ID: <47ef28d8-9936-6c70-e48e-1d9b5b334b5e@tarantool.org> Date: Sat, 16 Nov 2019 13:25:18 +0100 MIME-Version: 1.0 In-Reply-To: <20191116115421.GD14490@atlas> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH 1/1] iproto: don't destroy a session during disconnect List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Konstantin Osipov , tarantool-patches@dev.tarantool.org On 16/11/2019 12:54, Konstantin Osipov wrote: > * Vladislav Shpilevoy [19/11/16 12:48]: >> Binary session disconnect trigger yield could lead to use after >> free of the session object. That happened because iproto thread >> sent two requests to TX thread at disconnect: >> >> - Close the session and run its on disconnect triggers; >> >> - If all requests are handled, destroy the session. >> >> When a connection is idle, all requests are handled, so both these >> requests are sent. If the first one yielded in TX thread, the >> second one arrived and destroyed the session right under the feet >> of the first one. >> >> This can be solved in two ways - in TX thread, and in iproto >> thread. >> >> TX thread solution (which is chosen in the patch): add a flag >> which says whether disconnect is processed by TX. When destroy >> request arrives, it checks the flag. If disconnect is not done, >> the destroy request waits on a condition variable until it is. >> >> The solution is simple, but adds new members to iproto_connection >> struct, and requires lots of commenting. >> >> Iproto thread solution (alternative): just don't send destroy >> request until disconnect returns back to iproto thread. > > I like this one more to be honest. > Me too. Then look at v2 thread.