Re: [Tarantool-patches] [PATCH v15 07/11] module_cache: use references as a main usage counter

[Cyrill Gorcunov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On Sun, Feb 07, 2021 at 06:20:34PM +0100, Vladislav Shpilevoy wrote:
> Thanks for the patch!
> 
> See 2 comments below.
> 
> > +
> > +/**
> > + * Decrease module reference and delete it if last one.
> > + */
> > +static void
> > +module_unref(struct module *module)
> > +{
> > +	assert(module->refs > 0);
> > +	if (module->refs-- == 1) {
> > +		if (!module_is_orphan(module)) {
> > +			size_t len = strlen(module->package);
> > +			module_cache_del(module->package,
> > +					 &module->package[len]);
> 
> 1. You can also compare pointer in the hash. If it matches -
> delete it. Then you wouldn't need the orphan concept at all.

This gonna be a way more longer than zapping slash. But if you
prefer this way, sure thing.

> >  
> > -	module_gc(old);
> > +	module_set_orphan(old);
> > +	module_unref(old);
> > +
> > +	/* From explicit load above */
> > +	module_unref(new);
> 
> 2. Where is the explicit ref?

A couple of lines above

	struct module *new = module_load(package, package_end);
	if (new == NULL)
		return -1;

The thing is that box.schema.func doesn't use module_load at all
which takes first reference, instead it uses symols as an origin
for counters, thus we need to decrease the conter here because
when box.schema.func close last symbol nobody gonna call module_unload
and module will hang in memory forever. Probably I should write
more verbose comment here.

	Cyrill