gdb/python: deallocate tui window factories at Python shut down

The previous commit relied on spotting when a Python defined TUI
window factory was deleted.  I spotted that the window factories are
not deleted when GDB shuts down its Python environment, they are only
deleted when one window factory replaces another.  Consider this
example Python script:

  class TestWindowFactory:
      def __init__(self, msg):
          self.msg = msg
          print("Entering TestWindowFactory.__init__: %s" % self.msg)

      def __call__(self, tui_win):
          print("Entering TestWindowFactory.__call__: %s" % self.msg)
          return TestWindow(tui_win, self.msg)

      def __del__(self):
          print("Entering TestWindowFactory.__del__: %s" % self.msg)

  gdb.register_window_type("test_window", TestWindowFactory("A"))
  gdb.register_window_type("test_window", TestWindowFactory("B"))

And this GDB session:

  (gdb) source tui.py
  Entering TestWindowFactory.__init__: A
  Entering TestWindowFactory.__init__: B
  Entering TestWindowFactory.__del__: B
  (gdb) quit

Notice that when the 'B' window replaces the 'A' window we see the 'A'
object being deleted.  But, when Python is shut down (after the
'quit') the 'B' object is never deleted.

Instead, GDB retains a reference to the window factory object, which
forces the Python object to remain live even after the Python
interpreter itself has been shut down.

The references themselves are held in a dynamically allocated
std::unordered_map (in tui/tui-layout.c) which is never deallocated,
thus the underlying Python references are never decremented to zero,
and so GDB never tries to delete these Python objects.

This commit is the first half of the work to clean up this edge case.

All gdbpy_tui_window_maker objects (the objects that implement the
TUI window factory callback for Python defined TUI windows), are now
linked together into a global list using the intrusive list mechanism.

When GDB shuts down the Python interpreter we can now walk this global
list and release the reference that is held to the underlying Python
object.  By releasing this reference the Python object will now be
deleted.

I've added a new assert in gdbpy_tui_window_maker::operator(), this
will catch the case where we somehow end up in here after having
reset the reference to the underlying Python object.  I don't think
this should ever happen though as we only clear the references when
shutting down the Python interpreter, and the ::operator() function is
only called when trying to apply a new TUI layout - something that
shouldn't happen while GDB itself is shutting down.

This commit does not update the std::unordered_map in tui-layout.c,
that will be done in the next commit.

Reviewed-By: Tom Tromey <tom@tromey.com>
This commit is contained in:
Andrew Burgess 2023-01-12 15:47:17 +00:00
parent d159d87072
commit 9ae4519da9
4 changed files with 84 additions and 2 deletions

View file

@ -21,6 +21,7 @@
#include "defs.h"
#include "arch-utils.h"
#include "python-internal.h"
#include "gdbsupport/intrusive_list.h"
#ifdef TUI
@ -268,12 +269,14 @@ tui_py_window::output (const char *text, bool full_window)
user-supplied window constructor. */
class gdbpy_tui_window_maker
: public intrusive_list_node<gdbpy_tui_window_maker>
{
public:
explicit gdbpy_tui_window_maker (gdbpy_ref<> &&constr)
: m_constr (std::move (constr))
{
m_window_maker_list.push_back (*this);
}
~gdbpy_tui_window_maker ();
@ -281,12 +284,14 @@ public:
gdbpy_tui_window_maker (gdbpy_tui_window_maker &&other) noexcept
: m_constr (std::move (other.m_constr))
{
m_window_maker_list.push_back (*this);
}
gdbpy_tui_window_maker (const gdbpy_tui_window_maker &other)
{
gdbpy_enter enter_py;
m_constr = other.m_constr;
m_window_maker_list.push_back (*this);
}
gdbpy_tui_window_maker &operator= (gdbpy_tui_window_maker &&other)
@ -304,16 +309,43 @@ public:
tui_win_info *operator() (const char *name);
/* Reset the m_constr field of all gdbpy_tui_window_maker objects back to
nullptr, this will allow the Python object referenced to be
deallocated. This function is intended to be called when GDB is
shutting down the Python interpreter to allow all Python objects to be
deallocated and cleaned up. */
static void
invalidate_all ()
{
gdbpy_enter enter_py;
for (gdbpy_tui_window_maker &f : m_window_maker_list)
f.m_constr.reset (nullptr);
}
private:
/* A constructor that is called to make a TUI window. */
gdbpy_ref<> m_constr;
/* A global list of all gdbpy_tui_window_maker objects. */
static intrusive_list<gdbpy_tui_window_maker> m_window_maker_list;
};
/* See comment in class declaration above. */
intrusive_list<gdbpy_tui_window_maker>
gdbpy_tui_window_maker::m_window_maker_list;
gdbpy_tui_window_maker::~gdbpy_tui_window_maker ()
{
gdbpy_enter enter_py;
m_constr.reset (nullptr);
/* Remove this gdbpy_tui_window_maker from the global list. */
m_window_maker_list.erase (m_window_maker_list.iterator_to (*this));
if (m_constr != nullptr)
{
gdbpy_enter enter_py;
m_constr.reset (nullptr);
}
}
tui_win_info *
@ -332,6 +364,14 @@ gdbpy_tui_window_maker::operator() (const char *win_name)
std::unique_ptr<tui_py_window> window
(new tui_py_window (win_name, wrapper));
/* There's only two ways that m_constr can be reset back to nullptr,
first when the parent gdbpy_tui_window_maker object is deleted, in
which case it should be impossible to call this method, or second, as
a result of a gdbpy_tui_window_maker::invalidate_all call, but this is
only called when GDB's Python interpreter is being shut down, after
which, this method should not be called. */
gdb_assert (m_constr != nullptr);
gdbpy_ref<> user_window
(PyObject_CallFunctionObjArgs (m_constr.get (),
(PyObject *) wrapper.get (),
@ -572,3 +612,11 @@ gdbpy_initialize_tui ()
return 0;
}
/* Finalize this module. */
void
gdbpy_finalize_tui ()
{
gdbpy_tui_window_maker::invalidate_all ();
}

View file

@ -550,6 +550,7 @@ int gdbpy_initialize_unwind (void)
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
int gdbpy_initialize_tui ()
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
void gdbpy_finalize_tui ();
int gdbpy_initialize_membuf ()
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
int gdbpy_initialize_connection ()

View file

@ -1950,6 +1950,7 @@ finalize_python (void *ignore)
gdbpy_enter::finalize ();
gdbpy_finalize_micommands ();
gdbpy_finalize_tui ();
Py_Finalize ();