Skip to content

Commit

Permalink
mainwindow - Fix ‘RuntimeError: wrapped C/C++ object of type JLCPCBTo…
Browse files Browse the repository at this point in the history
…ols has been deleted’

To reproduce:
- Open kicad-jlcpcb-tools dialog
- Close dialog
- Re-open kicad-jlcpcb-tools dialog
- Attempt to download the database (this causes log messages to be generated), see RuntimeError dialog pop up.

Each time the mainwindow dialog is opened, init_logger() is called. init_logger() retrieves
the root logger via a call to logging.getLogger().

Handlers are then attached to the root logger, one to log to stderr and one to log via
the LogBoxHandler class to the wxWidgets textctrl log box at the bottom of the mainwindow dialog.

Recently there was a change to use wxPython events to log messages, to fix an issue that
was believed to be due to manipulation of the log output textctrl widget from the backgound
thread that was downloading the database.

This change removed a try/except that was discarding exceptions, as it was believed that this change
fixed the cause of the exceptions by marshalling, via wx.queueEvent(), the log text to the main
thread for addition to the log output textctrl widget.

Root cause is that the Python instance persists for the duration of Kicad's execution. Thus
calls to addHandler() to the root logger are cumulative. Each time you re-open the mainwindow
the logging handlers are added. The previous handlers, which now refer to deleted instances of
the mainwindow (class JLCPCBTools, see the error message in the commit subject), persist.
Each subsequent call to Python's logging functions results in all of these loggers being
called.

Previously the try/except in each of these loggers was discarding the wxPython/wxWidgets errors
but removing the try/except meant they were now unhandled.

Fix the crash by calling removeHandler() in quit_dialog() so there are no old logger handlers
with references to now deleted dialogs.

Note that due to the try/except it is likely that this issue was latent in the plugin for months
or years, at least since the addHandler() was called without removeHandler().

Special thanks to Aleksandr Shvartzkop took time to look at the code and almost immediately
spotted the logger addHandler() calls without corresponding removeHandler() calls. This was the
first step that led to debugging the issue.
  • Loading branch information
chmorgan authored and Bouni committed May 29, 2024
1 parent 8742c8c commit 8845133
Showing 1 changed file with 12 additions and 8 deletions.
20 changes: 12 additions & 8 deletions mainwindow.py
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,10 @@ def __init__(self, parent, kicad_provider=KicadProvider()):

def quit_dialog(self, *_):
"""Destroy dialog on close."""
root = logging.getLogger()
root.removeHandler(self.logging_handler1)
root.removeHandler(self.logging_handler2)

self.Destroy()
self.EndModal(0)

Expand Down Expand Up @@ -1096,19 +1100,19 @@ def init_logger(self):
root = logging.getLogger()
root.setLevel(logging.DEBUG)
# Log to stderr
handler1 = logging.StreamHandler(sys.stderr)
handler1.setLevel(logging.DEBUG)
self.logging_handler1 = logging.StreamHandler(sys.stderr)
self.logging_handler1.setLevel(logging.DEBUG)
# and to our GUI
handler2 = LogBoxHandler(self)
handler2.setLevel(logging.DEBUG)
self.logging_handler2 = LogBoxHandler(self)
self.logging_handler2.setLevel(logging.DEBUG)
formatter = logging.Formatter(
"%(asctime)s - %(levelname)s - %(funcName)s - %(message)s",
datefmt="%Y.%m.%d %H:%M:%S",
)
handler1.setFormatter(formatter)
handler2.setFormatter(formatter)
root.addHandler(handler1)
root.addHandler(handler2)
self.logging_handler1.setFormatter(formatter)
self.logging_handler2.setFormatter(formatter)
root.addHandler(self.logging_handler1)
root.addHandler(self.logging_handler2)
self.logger = logging.getLogger(__name__)

def __del__(self):
Expand Down

0 comments on commit 8845133

Please sign in to comment.