Skip to content

Commit

Permalink
[python/viewer] Do not use 'atexit' anymore to fix memory leaks.
Browse files Browse the repository at this point in the history
  • Loading branch information
duburcqa committed Dec 31, 2023
1 parent f73c6ce commit 9e2eea9
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 30 deletions.
4 changes: 0 additions & 4 deletions python/jiminy_py/src/jiminy_py/viewer/meshcat/wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import sys
import urllib
import base64
import atexit
import asyncio
import logging
import pathlib
Expand Down Expand Up @@ -341,9 +340,6 @@ def __init__(self,

self.comm_manager = CommManager(comm_url)

# Make sure the server is properly closed
atexit.register(self.close)

def __del__(self) -> None:
self.close()

Expand Down
38 changes: 12 additions & 26 deletions python/jiminy_py/src/jiminy_py/viewer/viewer.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import math
import shutil
import base64
import atexit
import logging
import pathlib
import tempfile
Expand Down Expand Up @@ -306,19 +305,17 @@ def fun_safe(*args: Any, **kwargs: Any) -> Any:
class _ProcessWrapper:
"""Wrap `multiprocessing.process.BaseProcess`, `subprocess.Popen`, and
`psutil.Process` in the same object to provide the same user interface.
It also makes sure that the process is properly terminated at Python exits,
and without zombies left behind.
"""
def __init__(self,
proc: Union[
multiprocessing.process.BaseProcess, subprocess.Popen,
Panda3dApp, Process],
kill_at_exit: bool = False):
Panda3dApp, Process]):
self._proc = proc
# Make sure the process is killed at Python exit
if kill_at_exit:
atexit.register(self.kill)

def __del__(self) -> None:
"""Automatically kill process at garbage collection.
"""
self.kill()

def is_parent(self) -> bool:
""" TODO: Write documentation.
Expand Down Expand Up @@ -657,10 +654,7 @@ def __init__(self, # pylint: disable=unused-argument
force_update_visual=True, force_update_collision=True, wait=True)

def __del__(self) -> None:
"""Destructor.
.. note::
It automatically close the viewer before being garbage collected.
"""Automatically close the viewer at garbage collection.
"""
self.close()

Expand Down Expand Up @@ -1108,7 +1102,6 @@ def close(self: Optional[Union["Viewer", Type["Viewer"]]] = None) -> None:
pass
Viewer._backend_proc.wait(0.2)
Viewer._backend_proc.kill()
atexit.unregister(Viewer.close)
Viewer.backend = None
Viewer._backend_obj = None
Viewer._backend_proc = None
Expand Down Expand Up @@ -1164,8 +1157,7 @@ def close(self: Optional[Union["Viewer", Type["Viewer"]]] = None) -> None:

@staticmethod
@_with_lock
def connect_backend(backend: Optional[str] = None,
close_at_exit: bool = True) -> None:
def connect_backend(backend: Optional[str] = None) -> None:
"""Get the running process of backend client.
This method can be used to open a new process if necessary.
Expand All @@ -1174,8 +1166,6 @@ def connect_backend(backend: Optional[str] = None,
'panda3d', 'panda3d-qt', 'meshcat'.
Optional: The default is hardware and environment
dependent. See `viewer.default_backend` for details.
:param close_at_exit: Terminate backend server at Python exit.
Optional: True by default
:returns: Pointer to the running backend Client and its PID.
"""
Expand Down Expand Up @@ -1211,14 +1201,14 @@ def connect_backend(backend: Optional[str] = None,
if backend == 'panda3d-qt':
from .panda3d.panda3d_widget import Panda3dQWidget
client = Panda3dQWidget()
proc = _ProcessWrapper(client, close_at_exit)
proc = _ProcessWrapper(client)
elif backend == 'panda3d-sync':
client = Panda3dApp()
proc = _ProcessWrapper(client, close_at_exit)
proc = _ProcessWrapper(client)
else:
client = Panda3dViewer(window_type='onscreen',
window_title=Viewer.window_name)
proc = _ProcessWrapper(client._app, close_at_exit)
proc = _ProcessWrapper(client._app)
except RuntimeError as e:
raise RuntimeError(
"Something went wrong. Impossible to instantiate viewer "
Expand Down Expand Up @@ -1294,7 +1284,7 @@ def connect_backend(backend: Optional[str] = None,
server_proc = Process(conn.pid)
else:
server_proc = client.server_proc
proc = _ProcessWrapper(server_proc, close_at_exit)
proc = _ProcessWrapper(server_proc)

# Make sure the backend process is alive
assert proc.is_alive(), (
Expand All @@ -1305,10 +1295,6 @@ def connect_backend(backend: Optional[str] = None,
Viewer._backend_obj = client
Viewer._backend_proc = proc

# Make sure to close cleanly the viewer at exit
if close_at_exit:
atexit.register(Viewer.close)

@staticmethod
@_with_lock
@_must_be_open
Expand Down

0 comments on commit 9e2eea9

Please sign in to comment.