Re: [Tarantool-patches] [PATCH v7 2/2] fiber: leak slab if unable to bring prots back

[Cyrill Gorcunov <gorcunov@gmail.com>]

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