[Tarantool-patches] [PATCH v20 4/7] box/module_cache: introduce modules subsystem

[Vladislav Shpilevoy]

>>> +void
>>> +module_free(void)
>>> +{
>>> +	mh_int_t e;
>>> +
>>> +	mh_foreach(module_cache, e) {
>>> +		struct module *m = mh_strnptr_node(module_cache, e)->val;
>>> +		module_unload(m);
>>
>> 5. As I said in the previous review, it does not make much sense.
>> If there are any not unloaded modules, and they try to unload later,
>> they will see module_cache == NULL and will crash.
>>
>> Also you can't do unload here, because the module_cache itself does
>> not keep any references. All the unloads must be done by the module
>> objects owners. Not by module_cache on its own. For example, if there
>> is a module having a single reference and used in some other subsystem,
>> your unload will free it and make it memory invalid. That will crash
>> in case the module owner will try to access it again.
>>
>> There should be a panic-check that the module cache is empty already.
> 
> Not at all. You can exit tarantool via Ctrl+D inside console and
> modules won't be empty and we should clean them up. So I can and
> I should unload modules here. Vlad, this is _exit_ path called when
> we're exiting tarantool. What I'm missing?

Well, if there are modules in Lua, they might have more than 1 reference,
and your module_unload won't free them anyway. But that does not matter
much as you try to free the objects which don't belong to you. The
refs do not belong to the module_cache subsystem. They belong to the
callers of module_load.

That is a bug. Freeing what does not belong to you.

And since the Lua land is not properly terminated with freeing all the
references, the only valid way you have here is not to do anything at
all AFAIS. Or free the hash table + set it to NULL so it would at least
crash in a sane way in case we ever start freeing all Lua refs. But
under no circumstances can you unref the modules which you didn't ref.
They were referenced by schema modules and box.lib modules, and
therefore must be unreferenced by them.