From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <korablev@tarantool.org>
Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149])
 (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 E8E7F469719
 for <tarantool-patches@dev.tarantool.org>;
 Thu, 20 Feb 2020 14:53:52 +0300 (MSK)
Date: Thu, 20 Feb 2020 14:53:51 +0300
From: Nikita Pettik <korablev@tarantool.org>
Message-ID: <20200220115351.GE40100@tarantool.org>
References: <cover.1582119629.git.korablev@tarantool.org>
 <fc1e8f3955c9fe2954f6e2569a9981b0066aad68.1582119629.git.korablev@tarantool.org>
 <c32161ff-ad28-3f88-022a-a6c0ec617a74@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <c32161ff-ad28-3f88-022a-a6c0ec617a74@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH 4/7] box: introduce stacked
 diagnostic area
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org

On 19 Feb 22:10, Vladislav Shpilevoy wrote:
> 
> 
> On 19/02/2020 15:16, Nikita Pettik wrote:
> > @@ -163,7 +221,12 @@ diag_clear(struct diag *diag)
> >  {
> >  	if (diag->last == NULL)
> >  		return;
> > -	error_unref(diag->last);
> > +	struct error *last = diag->last;
> > +	while (last != NULL) {
> > +		struct error *tmp = last->next;
> > +		error_unref(last);
> > +		last = tmp;
> > +	}
> >  	diag->last = NULL;
> 
> Hi! Please, read what I wrote in the ticket about box.error.new(). You
> should not clear all the errors. The diag owns only the head ref. Head
> destruction should unref a next error. Destruction of a next error
> should unref next next error and so on.

Hi,

I've realized that current approach is a bit broken only by now.
You are right: diag_clear() should unref only head  meanwhile error
destruction should result in decrementing reference counter of previous
error. Just for the record why my implementation doesn't work correct:

e1 = box.error.new(...) -- e1 has 1 ref
e2 = box.error.new(...) -- e2 has 1 ref

e1:set_prev(e2)
box.error.set(e1) -- e1 has 2 ref as a head of diag, e2 still has 1 ref
box.error.clear() -- e1 has 1 ref, but e2 has 0 so it is destroyed.
-- However, it still has Lua reference, so when GC will attempt to
-- destroy it again, it will lead to crash.

Will fix it soon, re-send patch and update rfc.