From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f195.google.com (mail-lj1-f195.google.com [209.85.208.195]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 87565469719 for ; Fri, 14 Feb 2020 11:25:21 +0300 (MSK) Received: by mail-lj1-f195.google.com with SMTP id q8so9692933ljj.11 for ; Fri, 14 Feb 2020 00:25:21 -0800 (PST) Date: Fri, 14 Feb 2020 11:25:18 +0300 From: Cyrill Gorcunov Message-ID: <20200214082518.GU21061@uranus> References: <20200213205618.7982-1-gorcunov@gmail.com> <20200213205618.7982-3-gorcunov@gmail.com> <2291dd87-04c7-dd92-d8aa-e56ad46e4681@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2291dd87-04c7-dd92-d8aa-e56ad46e4681@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v7 2/2] fiber: leak slab if unable to bring prots back List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tml On Fri, Feb 14, 2020 at 12:26:04AM +0100, Vladislav Shpilevoy wrote: > > --- a/test/unit/fiber_stack.c > > +++ b/test/unit/fiber_stack.c > > @@ -59,6 +58,30 @@ main_f(va_list ap) > > ok(fiber != NULL, "madvise: non critical error on madvise hint"); > > ok(diag_get() != NULL, "madvise: diag is armed after error"); > > > > + /* > > + * Check if we leak on fiber descrution. > > 1. You probably have meant 'destruction'. Sharp eyes =) Thanks! > > + > > + fiber_attr_delete(fiber_attr); > > 2. When I run the test without the fix, I got 'Bus error'. > Was it intended? In my email I proposed to compare memory Hmm. Actually not, should not be. I'll dig into, thanks! > statistics, but crash also looks ok, maybe. If this is what > you meant. And then there should be a comment saying, that we > expect fiber_join() to crash when no leak is done, because it > would put the slab into the cash, where it would be read/written > against protection. This was not obvious until I reverted your > patch and run the test. I'll try to play with statistics again, the error message was just simplier and faster. > IMO, problem of that way of testing is that if internals of > core/fiber.c one day will stop touching stack of a just > destroyed fiber (for example, a pointer at it would be cached > somewhere for newer fibers), this test will happily pass, even > though it perhaps should not. It will pass regardless of what > happens with a fiber stack unless something tries to read/write > it. Thanks for comments! Cyrill