Re: [Tarantool-patches] [PATCH 2/2] func: fix not unloading of unused modules

[Konstantin Osipov <kostja.osipov@gmail.com>]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/11/27 11:15]:
> C functions are loaded from .so/.dylib dynamic libraries. A
> library is loaded when any function from there is called first
> time. And was supposed to be unloaded, when all its functions are
> dropped from the schema (box.schema.func.drop()), and none of them
> is still in a call. But the unloading part was broken.

Wow. This is damn genius!

It never occurred to me that error injections could be used to
test some obscure code paths.

The impact of this on testing is going to be huge.
>  module_gc(struct module *module)
>  {
> -	if (!module->is_unloading || !rlist_empty(&module->funcs) ||
> -	     module->calls != 0)
> -		return;
> -	module_delete(module);
> +	if (rlist_empty(&module->funcs) && module->calls == 0)
> +		module_delete(module);
>  }

This is LGTM. Georgy wrote the original patches so you may want
to solicit his review as well.
> +f1:wakeup()
> + | ---
> + | ...
> +test_run:wait_cond(function() return f1:status() == 'dead' end)
> + | ---
> + | - true
> + | ...
> +check_module_count_diff(0)
> + | ---
> + | ...
> +
> +f2:wakeup()

This is a bit hackish way to cancel a sleep. I thought we had
errinj_sleep injection which would reliably cancel a sleep when
you clear it? If there is no such function I don't think 
it is a blocker for this patch to get in as is.

-- 
Konstantin Osipov, Moscow, Russia