Avoid passing a parent to QProcess

With the upgrade to MarkupSafe 3.0, something funny happened when trying to pass
the GUIProcess object to jinja after launching a userscript:

    [...]
    File "[...]/qutebrowser/browser/qutescheme.py", line 291, in qute_process
        src = jinja.render('process.html', title=f'Process {pid}', proc=proc)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "[...]/qutebrowser/utils/jinja.py", line 123, in render
        return environment.get_template(template).render(**kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    [...]
    File "html/process.html", line 11, in block 'content'
    File "[...]/lib/python3.11/site-packages/markupsafe/__init__.py", line 42, in escape
        if hasattr(s, "__html__"):
           ^^^^^^^^^^^^^^^^^^^^^^
    RuntimeError: wrapped C/C++ object of type GUIProcess has been deleted

This can be reproduced with:

    qutebrowser --temp-basedir ':cmd-later 0 spawn -u -o /bin/echo test'

We pass the `GUIProcess` to the Jinja template as `proc`, which then formats it as
`{{ proc }}`` (to stringify it). For some reason, with the newest MarkupSafe/Jinja
versions, this now triggers the `if hasattr(s, "__html__")` check in MarkupSafe
(which has been around for a while). That then presumably causes PyQt to try and
access the underlying C++ object for `GUIProcess``, but that has already been
deleted.

But why is it deleted in the first place, if we keep track of even completed
processes data ever since we added `:process` in a3adba81c? It looks like the Qt
parent-child relationship is the culprit here: When we pass a parent to the
`GUIProcess`` from the userscript runner, it will get deleted as soon as said
runner is cleaned up (which happens after the userscript has finished).

We probably never noticed this before because we only accessed data from the
Python wrapper and not from the C++ side, but it still seems like a good idea
to avoid passing a parent for a long-lived object (with time-based cleanup) in
the first place.
This commit is contained in:
Florian Bruhin 2024-10-13 19:08:42 +02:00
parent f175f611f8
commit ff5d4d3564
5 changed files with 12 additions and 6 deletions

View File

@ -44,6 +44,12 @@ Changed
- Updated mimetype information for getting a suitable extension when downloading
a `data:` URL.
Fixed
~~~~~
- Crash with recent Jinja/Markupsafe versions when viewing a finished userscript
(or potentially editor) process via `:process`.
[[v3.3.1]]
v3.3.1 (2024-10-12)

View File

@ -1137,8 +1137,7 @@ class CommandDispatcher:
else:
cmd = os.path.expanduser(cmd)
proc = guiprocess.GUIProcess(what='command', verbose=verbose,
output_messages=output_messages,
parent=self._tabbed_browser)
output_messages=output_messages)
if detach:
ok = proc.start_detached(cmd, args)
if not ok:

View File

@ -156,7 +156,7 @@ class _BaseUserscriptRunner(QObject):
self.proc = guiprocess.GUIProcess(
'userscript', additional_env=self._env,
output_messages=output_messages, verbose=verbose, parent=self)
output_messages=output_messages, verbose=verbose)
self.proc.finished.connect(self.on_proc_finished)
self.proc.error.connect(self.on_proc_error)
self.proc.start(cmd, args)

View File

@ -180,7 +180,7 @@ class ExternalEditor(QObject):
line: the line number to pass to the editor
column: the column number to pass to the editor
"""
self._proc = guiprocess.GUIProcess(what='editor', parent=self)
self._proc = guiprocess.GUIProcess(what='editor')
self._proc.finished.connect(self._on_proc_closed)
self._proc.error.connect(self._on_proc_error)
editor = config.val.editor.command

View File

@ -177,9 +177,10 @@ class GUIProcess(QObject):
verbose: bool = False,
additional_env: Mapping[str, str] = None,
output_messages: bool = False,
parent: QObject = None,
):
super().__init__(parent)
# We do not accept a parent, as GUIProcesses keep track of themselves
# (see all_processes and _post_start() / _on_cleanup_timer())
super().__init__()
self.what = what
self.verbose = verbose
self._output_messages = output_messages