Re: [Tarantool-patches] [PATCH 4/7] box: introduce stacked diagnostic area

[Nikita Pettik <korablev@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.