From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id CE4273854386 for ; Thu, 12 Jan 2023 19:19:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CE4273854386 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1673551174; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Vk2Rkx1B8DZKpIGjO7T4A/mUnIR8hUX6y9zgUA24R/s=; b=XPwCUTJVC4Ng/oHk/QFL21ENTcj2nTo/sv4Ah0z9Mseh34euB8D2WPvMxUGEsSU0A7i/qi VN1XZRMuYm9OWgGyUNVRUd7tCO9LRHnprRpbg+gGUXwEKoMdw+zVfRJDik+aFru8IfpLzZ +HlXGkmdVqZdbWhmfwQR00k3K7d3CI4= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-244-Qad1XKSsP7GmxHQcjz2F0g-1; Thu, 12 Jan 2023 14:19:33 -0500 X-MC-Unique: Qad1XKSsP7GmxHQcjz2F0g-1 Received: by mail-wm1-f69.google.com with SMTP id fm17-20020a05600c0c1100b003d96f0a7f2eso12917246wmb.6 for ; Thu, 12 Jan 2023 11:19:33 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Vk2Rkx1B8DZKpIGjO7T4A/mUnIR8hUX6y9zgUA24R/s=; b=aD2nsJbNJvkwptzWl6NrMHAxMstiHfm0BXg+YzTG6pIZTZRvRTZyRnxVs5Va/TkC0X ClzGLKOkNwmuepUCQpijIbqUWmpYwn/22s7DaMUDLFTgf9qDgsKMX8PdWtCvq59pEjZY aFiMLj9GXQur31Pzt+rB7IQvd87jh19l45FnWRTk4BCVcqCHEkyAdmYnwak6Jn8KRWsD EKIl7BTbdL6lBO/49Ppl6bq5Ydxay/ONAvZasAMmAKrOuKIL3a0bbKVhOvGf61bRKH7m PrF60CT2gr99m04bK2yHdYUeIgk4gRGVbUDPnANjVA7Tq6OJS6VeKaitl1s0Am4XNCk8 FluA== X-Gm-Message-State: AFqh2kobPU4PF4LeCImQS/yTKgk5Nexsjmo3BH/opHolQ2pYnDRUeJay 2uQmQLSjd4g+jW99o3qS994xwXOqiGZ5FBOKf2RNQ4l0A2N22nqwOI6WX7KppYmq4d1Zdry3DXR HrIt9ZoS7HvNiRrbwQyYPgnTq8NuCXg+GanOiwOdLJoGL4+OzABlNKzzsrbss+NqyA8BLGmEqsQ == X-Received: by 2002:a5d:6049:0:b0:2a6:4dfb:80c8 with SMTP id j9-20020a5d6049000000b002a64dfb80c8mr21600361wrt.19.1673551171945; Thu, 12 Jan 2023 11:19:31 -0800 (PST) X-Google-Smtp-Source: AMrXdXvanX92LslBFH1wmu7LaRqf7hRCo+7EPLcM3aC6rd2qMiJjN4towRhiFQpO7OpIDqEzAf5Vkw== X-Received: by 2002:a5d:6049:0:b0:2a6:4dfb:80c8 with SMTP id j9-20020a5d6049000000b002a64dfb80c8mr21600349wrt.19.1673551171658; Thu, 12 Jan 2023 11:19:31 -0800 (PST) Received: from localhost (92.40.218.34.threembb.co.uk. [92.40.218.34]) by smtp.gmail.com with ESMTPSA id o14-20020a5d58ce000000b002879c013b8asm16981542wrf.42.2023.01.12.11.19.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Jan 2023 11:19:31 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 2/3] gdb/python: deallocate tui window factories at Python shut down Date: Thu, 12 Jan 2023 19:19:21 +0000 Message-Id: <01961fb5bffc73841bc9782b819e19d2be5c039f.1673550880.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: The previous commit relied on detecting when a TUI window factory (as defied in Python) was deleted. I spotted that the window factories are not deleted when GDB shuts down its Python environment. Instead, a reference to these window factories is retained, which forces the Python object to remain live even after the Python interpreter itself has been shut down. We get away with this because 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. This commit is the first half of the work to clean up this edge case. All gdbpy_tui_window_maker objects, the objects which implement the TUI window factory callback for Python defined TUI windows, are added to a global list which lives in py-tui.c. When GDB shuts down the Python interpreter we now invalidate all of the existing gdbpy_tui_window_maker objects (by releasing the reference to the underlying Python object). As a result, when GDB shuts down the Python interpreter, all the existing TUI window factory Python objects are deleted. This commit does not update the std::unordered_map in tui-layout.c, that will be done in the next commit. --- gdb/python/py-tui.c | 52 ++++++++++++++++++- gdb/python/python-internal.h | 1 + gdb/python/python.c | 1 + .../gdb.python/tui-window-factory.exp | 32 ++++++++++++ 4 files changed, 84 insertions(+), 2 deletions(-) diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c index e9c91012ae9..9ce76659052 100644 --- a/gdb/python/py-tui.c +++ b/gdb/python/py-tui.c @@ -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 { 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 @@ class gdbpy_tui_window_maker 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 @@ class gdbpy_tui_window_maker 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 m_window_maker_list; }; +/* See comment in class declaration above. */ + +intrusive_list + 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 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 (); +} diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h index c41a43bac96..8fb09796b15 100644 --- a/gdb/python/python-internal.h +++ b/gdb/python/python-internal.h @@ -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 () diff --git a/gdb/python/python.c b/gdb/python/python.c index 9d558119e09..3f5169251fc 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -1932,6 +1932,7 @@ finalize_python (void *ignore) gdbpy_enter::finalize (); gdbpy_finalize_micommands (); + gdbpy_finalize_tui (); Py_Finalize (); diff --git a/gdb/testsuite/gdb.python/tui-window-factory.exp b/gdb/testsuite/gdb.python/tui-window-factory.exp index b2d6153c947..e18392b3f51 100644 --- a/gdb/testsuite/gdb.python/tui-window-factory.exp +++ b/gdb/testsuite/gdb.python/tui-window-factory.exp @@ -78,3 +78,35 @@ with_test_prefix "msg_3" { Term::check_box_contents "check test_window box" 0 0 80 15 \ "TestWindow \\(msg_3\\)" } + +# Restart GDB, setup a TUI window factory, and then check that the +# Python object is deallocated when GDB exits. +with_test_prefix "call __del__ at exit" { + clean_restart + + gdb_test "source ${pyfile}" "Python script imported" \ + "import python scripts" + + gdb_test "python register_window_factory('msg_1')" \ + "Entering TestWindowFactory\\.__init__: msg_1" + + gdb_test "python register_window_factory('msg_2')" \ + [multi_line \ + "Entering TestWindowFactory\\.__init__: msg_2" \ + "Entering TestWindowFactory\\.__del__: msg_1"] + + set saw_window_factory_del 0 + gdb_test_multiple "quit" "" { + -re "^quit\r\n" { + exp_continue + } + -re "^Entering TestWindowFactory.__del__: msg_2\r\n" { + incr saw_window_factory_del + exp_continue + } + eof { + gdb_assert { $saw_window_factory_del == 1 } + pass $gdb_test_name + } + } +} -- 2.25.4