Plugins run twice in autoreload mode

[shniubobo]

• I have read the Filing Issues and subsequent “How to Get Help” sections of the documentation.
• I have searched the issues (including closed ones) and believe that this is not a duplicate.

• OS version and name: Ubuntu 18.04.5 LTS (WSL)
• Python version: 3.8.6
• Pelican version: e4d9c41

To reproduce

Create these files:

.
├── content
│   └── 1.rst
├── pelicanconf.py
└── plugins
    └── test_plugin.py

.. content/1.rst
####
test
####

:date: 1970-01-01
:modified: 1970-01-01

TEST

# pelicanconf.py
PLUGINS = [
    'test_plugin',
]
PLUGIN_PATHS = [
    'plugins',
]

# plugins/test_plugin.py
import logging

from pelican import signals

logger = logging.getLogger(__name__)


def test_function(content):
    logger.info('test plugin loaded')
    test = content._content
    test += 'TEST'
    content._content = test
    logger.info(content._content)


def register():
    signals.content_object_init.connect(test_function)

Then run:

$ pelican -rD --logs-dedup-min-level DEBUG content
[Not showing unrelated logs]
-> test plugin loaded
-> <p>TEST</p>
  | TEST
-> test plugin loaded
-> <p>TEST</p>
  | TESTTEST

$ pelican -D --logs-dedup-min-level DEBUG content
[Not showing unrelated logs]
-> test plugin loaded
-> <p>TEST</p>
  | TEST

The plugin runs twice with -r, but only once without -r.

The casue of the issue

After a bisect, I found that commit ed1eca1 introduced this issue.

$ git checkout ed1eca16^
$ pelican -rD --logs-dedup-min-level DEBUG content
[Not showing unrelated logs]
-> test plugin loaded
-> <p>TEST</p>
  | TEST

$ git checkout ed1eca16
$ pelican -rD --logs-dedup-min-level DEBUG content
[Not showing unrelated logs]
-> test plugin loaded
-> <p>TEST</p>
  | TEST
-> test plugin loaded
-> <p>TEST</p>
  | TESTTEST

After adding this line, I found that the plugin was registered twice:

diff --git a/pelican/contents.py b/pelican/contents.py
index 594cd3b5..ff991be9 100644
--- a/pelican/contents.py
+++ b/pelican/contents.py
@@ -139,6 +139,7 @@ class Content(object):
         if 'summary' in metadata:
             self._summary = metadata['summary']

+        logger.info(str(signals.content_object_init.receivers))
         signals.content_object_init.send(self)

     def __str__(self):

$ pelican -rD --logs-dedup-min-level DEBUG content
[Not showing unrelated logs]
-> {140542259499216: <weakref at 0x7fd28a2b8b80; to 'function' at 0x7fd28b7410d0 (test_function)>, 140542259600400: <weakref at 0x7fd28b7525e0; to 'function' at 0x7fd28b759c10 (test_function)>}
-> test plugin loaded
-> <p>TEST</p>
  | TEST
-> test plugin loaded
-> <p>TEST</p>
  | TESTTEST

But why? The reason:

>>> # Implementation of ed1eca16^
>>> import sys
>>> sys.path.insert(0, 'plugins')
>>> plugin = __import__('test_plugin', globals(), locals(), str('module'))
>>> plugin_ = __import__('test_plugin', globals(), locals(), str('module'))
>>> plugin is plugin_
True
>>>
>>> # Implementation of ed1eca16
>>> import importlib
>>> spec = importlib.machinery.PathFinder.find_spec('test_plugin', ['plugins'])
>>> plugin = importlib.util.module_from_spec(spec)
>>> plugin_ = importlib.util.module_from_spec(spec)
>>> plugin is plugin_
False

How to fix

To fix it, simply avoid loading the same plugin twice:

diff --git a/pelican/plugins/_utils.py b/pelican/plugins/_utils.py
index 4e6ec3c5..699192d3 100644
--- a/pelican/plugins/_utils.py
+++ b/pelican/plugins/_utils.py
@@ -53,6 +53,11 @@ def load_legacy_plugin(plugin, plugin_paths):
     if spec is None:
         raise ImportError('Cannot import plugin `{}`'.format(plugin))
     else:
+        # Avoid loading the same plugin twice
+        try:
+            return sys.modules[spec.name]
+        except KeyError:
+            pass
         # create module object from spec
         mod = importlib.util.module_from_spec(spec)
         # place it into sys.modules cache

I will create a pull request after writing some tests for it.

[avaris]

Thanks. I would expect old module to be deleted/garbage collected since it is overridden in the sys.modules. Returning module if it is already present makes sense, but try/except is a bit unnecessary. A simple if would do:

if spec.name in sys.modules:
    return sys.modules[spec.name]

[shniubobo]

Thanks. I would expect old module to be deleted/garbage collected since it is overridden in the sys.modules. Returning module if it is already present makes sense, but try/except is a bit unnecessary. A simple if would do:

if spec.name in sys.modules:
    return sys.modules[spec.name]

I used try/except to make it consistent with:

pelican/pelican/plugins/_utils.py

Lines 65 to 69 in e4d9c41

	try:
	# remove module from sys.modules since it can't be loaded
	del sys.modules[spec.name]
	except KeyError:
	pass

I will use if instead in my commit.

[justinmayer]

Fix for this issue is included in the just-released Pelican 4.5.1. ✨