pytest-dev/pluggy

`PluginManager.subset_hook_caller` can cause high memory usage

bluetech opened this issue · 3 comments

subset_hook_caller creates a HookManager which excludes calls to a set of a plugins.

A major user of subset_hook_caller is pytest, for its conftest mechanism - conftest plugins not relevant for a given path are excluded. pytest calls subset_hook_caller quite a lot, usually using the HookCaller and immediately throwing it away. In pytest, the lifetime of the subset HookCaller is much shorter than the PluginManager.

pluggy however needs to handle the following scenario:

  1. subset_hook_caller is called: shc = pm.subset_hook_caller(..)
  2. A certain (non-excluded) plugin is unregistered: pm.unregister(plugin).
  3. The subset HookCaller is called: shc.mymethod(...).

In step 3, plugin must not be called since it was unregistered in step 2. This means that the subset HookCaller must still be attached to the PluginManager somehow. This is done using a _plugin2hookcallers dict. The subset HookCaller adds itself to this dict:

# we also keep track of this hook caller so it
# gets properly removed on plugin unregistration
self._plugin2hookcallers.setdefault(plugin, []).append(hc)

Alas, once the hookcaller is added to this dict, it is never removed; hence the memory is only released when the entire PluginManager is gone, not when the lifetime of just the subset HookCaller is done.

Good catch. Perhaps we can use a context manager to remove the temporary hook caller from the dict?

This would have been nice, though this would require new API and might be a bit hard to use? I have an alternative proposal which is a little ugly but solves the problem I think -- will submit a PR soon (just testing it out against pytest).

This would have been nice, though this would require new API and might be a bit hard to use?

Right, it depends on usage, and looking at how pytest uses it indeed would not be useful.

will submit a PR soon (just testing it out against pytest).

Nice!