From patchwork Thu Jan 12 19:19:20 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 63117 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 418BF3857C44 for ; Thu, 12 Jan 2023 19:20:01 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 418BF3857C44 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1673551201; bh=poH6YMV6i906eiv3rHK7bvMhvQq6pfGmjwcyzO64sEo=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=lsKXJj7+OtGBvWbOWFkQt0xOCJEAKIsKkyCWJhNnYprjXLUie2lmh069PKbxQfhZv zQXdvy0IhynuavjKs7OuW4kgwv4Clo0uWPFdoLHbIPmKONnSWCGr0YiiI7AFg0yikW Hi91wXEIhLy5OAxvBAAHH+ZHtb3v0vmM7HpmKLSQ= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 069DF3858C74 for ; Thu, 12 Jan 2023 19:19:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 069DF3858C74 Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-575-7qXl1tduM66om5Eyp4VYjQ-1; Thu, 12 Jan 2023 14:19:31 -0500 X-MC-Unique: 7qXl1tduM66om5Eyp4VYjQ-1 Received: by mail-wr1-f69.google.com with SMTP id r10-20020adfa14a000000b0025ba73dff40so3781036wrr.12 for ; Thu, 12 Jan 2023 11:19:31 -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=poH6YMV6i906eiv3rHK7bvMhvQq6pfGmjwcyzO64sEo=; b=qz26bYyNR2gkxmK8K9xbhKlLNM9b80ByFxhNnmc+YgPiEwxfPhMKhNESTqFPs2i5T7 fKg7fbw6HC5xYwV7F+P2v35yP7F4Oqdnv1fXn84xKbiIPL53VpDPVIYjy6HnzcwXKVCD Acg2Xa9eRRgqWhx78r1u2NMlPPzeDvmp0cFwALi0sbQJ1591MqcM8QJXFPXrKBvOnWxY mQonwj1wsAZUBW4bzV27gm23LfUnnAluHBb3SZZ/rqFyRqaYcmNEbxgSy+75JAV4S3E6 kSFOqDB53IsCY2Fa93nR/eTrNs1cNg9n0iASoiKrIzhur5tO4HT78CLxBU15632uSxA9 Hd3Q== X-Gm-Message-State: AFqh2krLJVxPc5eZX1SZer7DoVfdny6bTbfl8FTHsImNncv+ffp5mXY6 XQ/Ahl9GLDFQ4DKRhfrjY09oWejDHv1tE9bo46imFVQyVeHDx87N0s2+BIaXn8msa3pHMRVt44Z OBlQU0uBHiXWwtSK0xGKUb2wwK9qISyvUIuXiRkXpszTWcP8SWPMwNWsWMrVEDC400+HJ6mZ28g == X-Received: by 2002:a05:600c:4f83:b0:3d2:3f55:f73f with SMTP id n3-20020a05600c4f8300b003d23f55f73fmr57033044wmq.8.1673551170190; Thu, 12 Jan 2023 11:19:30 -0800 (PST) X-Google-Smtp-Source: AMrXdXsTNZMS9v4dFx1jG9QWK9cNa3YRn6aenwbGHa6S/TbUcowSXub45IxasxcKIn87eq9bTLRoDg== X-Received: by 2002:a05:600c:4f83:b0:3d2:3f55:f73f with SMTP id n3-20020a05600c4f8300b003d23f55f73fmr57033031wmq.8.1673551169894; Thu, 12 Jan 2023 11:19:29 -0800 (PST) Received: from localhost (92.40.218.34.threembb.co.uk. [92.40.218.34]) by smtp.gmail.com with ESMTPSA id o5-20020a05600c510500b003b4ff30e566sm9789778wms.3.2023.01.12.11.19.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Jan 2023 11:19:29 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 1/3] gdb/python: allow Python TUI windows to be replaced Date: Thu, 12 Jan 2023 19:19:20 +0000 Message-Id: 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 X-Spam-Status: No, score=-11.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Andrew Burgess via Gdb-patches From: Andrew Burgess Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" The documentation for gdb.register_window_type says: "... It's an error to try to replace one of the built-in windows, but other window types can be replaced. ..." I take this to mean that if I imported a Python script like this: gdb.register_window_type('my_window', FactoryFunction) Then GDB would have a new TUI window 'my_window', which could be created by calling FactoryFunction(). If I then, in the same GDB session imported a script which included: gdb.register_window_type('my_window', UpdatedFactoryFunction) Then GDB would replace the old 'my_window' factory with my new one, GDB would now call UpdatedFactoryFunction(). This is pretty useful in practice, as it allows users to iterate on their window implementation within a single GDB session. However, right now, this is not how GDB operates. The second call to register_window_type is basically ignored and the old window factory is retained. This is because in tui_register_window (tui/tui-layout.c) we use std::unordered_map::emplace to insert the new factory function, and emplace doesn't replace an existing element in an unordered_map. In this commit, before the emplace call, I now search for an already existing element, and delete any matching element from the map, the emplace call will then add the new factory function. --- .../gdb.python/tui-window-factory.exp | 80 +++++++++++++++++++ .../gdb.python/tui-window-factory.py | 48 +++++++++++ gdb/tui/tui-layout.c | 8 ++ 3 files changed, 136 insertions(+) create mode 100644 gdb/testsuite/gdb.python/tui-window-factory.exp create mode 100644 gdb/testsuite/gdb.python/tui-window-factory.py diff --git a/gdb/testsuite/gdb.python/tui-window-factory.exp b/gdb/testsuite/gdb.python/tui-window-factory.exp new file mode 100644 index 00000000000..b2d6153c947 --- /dev/null +++ b/gdb/testsuite/gdb.python/tui-window-factory.exp @@ -0,0 +1,80 @@ +# Copyright (C) 2023 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test that GDB correctly deallocates the window factory object (a) +# when a window factory is replaced, and (b) during GDB shutdown. +# +# This test also ensures that when a new window is registered (via the +# Python API) with the same name as an existing window, then the +# previous window is replaced. + +load_lib gdb-python.exp + +tuiterm_env + +clean_restart + +if {[skip_tui_tests]} { + return +} + +if { [skip_python_tests] } { + untested "skipping Python tests" + return +} + +set pyfile [gdb_remote_download host \ + ${srcdir}/${subdir}/${gdb_test_file_name}.py] + +Term::clean_restart 24 80 +Term::prepare_for_tui + +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"] + +gdb_test_no_output "tui new-layout test test_window 1 cmd 1 status 1" + +# Load the custom window layout and ensure that the correct window +# factory was used. +with_test_prefix "msg_2" { + Term::command_no_prompt_prefix "layout test" + Term::check_box_contents "check test_window box" 0 0 80 15 \ + "TestWindow \\(msg_2\\)" +} + +# Replace the existing window factory with a new one, then switch +# layouts so that GDB recreates the window, and check that the new +# window factory was used. +with_test_prefix "msg_3" { + Term::command "python register_window_factory('msg_3')" + Term::check_region_contents "check for python output" \ + 0 18 80 2 \ + [multi_line \ + "Entering TestWindowFactory.__init__: msg_3\\s+" \ + "Entering TestWindowFactory.__del__: msg_2"] + Term::command "layout src" + Term::command "layout test" + + Term::check_box_contents "check test_window box" 0 0 80 15 \ + "TestWindow \\(msg_3\\)" +} diff --git a/gdb/testsuite/gdb.python/tui-window-factory.py b/gdb/testsuite/gdb.python/tui-window-factory.py new file mode 100644 index 00000000000..d1254e7e3a0 --- /dev/null +++ b/gdb/testsuite/gdb.python/tui-window-factory.py @@ -0,0 +1,48 @@ +# Copyright (C) 2023 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + + +class TestWindow: + def __init__(self, tui_win, msg): + self.msg = msg + self.tui_win = tui_win + print("Entering TestWindow.__init__: %s" % self.msg) + + def render(self): + self.tui_win.erase() + self.tui_win.write("TestWindow (%s)" % self.msg) + + def __del__(self): + print("Entering TestWindow.__del__: %s" % self.msg) + + +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) + + +def register_window_factory(msg): + gdb.register_window_type("test_window", TestWindowFactory(msg)) + + +print("Python script imported") diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c index 27abee02087..b895e00a80d 100644 --- a/gdb/tui/tui-layout.c +++ b/gdb/tui/tui-layout.c @@ -418,6 +418,14 @@ tui_register_window (const char *name, window_factory &&factory) if (!ISALPHA (name_copy[0])) error (_("window name must start with a letter, not '%c'"), name_copy[0]); + /* We already check above for all the builtin window names. If we get + this far then NAME must be a user defined window. Remove any existing + factory and replace it with this new version. */ + + auto iter = known_window_types->find (name); + if (iter != known_window_types->end ()) + known_window_types->erase (iter); + known_window_types->emplace (std::move (name_copy), std::move (factory)); } From patchwork Thu Jan 12 19:19:21 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 63119 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EE36738543AB for ; Thu, 12 Jan 2023 19:20:28 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EE36738543AB DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1673551229; bh=Vk2Rkx1B8DZKpIGjO7T4A/mUnIR8hUX6y9zgUA24R/s=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=svdFbqZw58uYHthUtZ46+ZFhJZRc3ADogV0atPLwVzvqH7NsdQcTVs4+j/tZ9tLh2 kQh8bl84axQ2qHvv55S7eroHMLrW2eUBbG6W+1bf+495rKYfSCiPq/brw05n+KGLzs CX6hXbU0gqhvBHEeKJIf06BvQ+8eH3gdd8JUPvfo= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org 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 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) 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 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Andrew Burgess via Gdb-patches From: Andrew Burgess Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" 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 + } + } +} From patchwork Thu Jan 12 19:19:22 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 63118 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 40D1038493DB for ; Thu, 12 Jan 2023 19:20:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 40D1038493DB DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1673551205; bh=fVn5IOLw5BZIPWuRa0Az7BeeqKdJycOl4yREX59G6Ig=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=OO6SSSirIN/jjI5LZYUYmAW2bNAIvNMlb5wrEvYwrydwtvGuQ1wNj2OxveUjNnles B3MC4eIHwQ+6RnuRy6TpsXeuSFn2wVRzINCVFhTywzeFWksH4FnnWzUxnPGczn4bY7 su1KDLCRPYncmDywT0mFI8bDQu8Wad+TbU+E/9wI= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org 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 3AFF3385B509 for ; Thu, 12 Jan 2023 19:19:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3AFF3385B509 Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-226-b6BGAw4RO2GAcayEbw7x1Q-1; Thu, 12 Jan 2023 14:19:34 -0500 X-MC-Unique: b6BGAw4RO2GAcayEbw7x1Q-1 Received: by mail-wm1-f71.google.com with SMTP id bi11-20020a05600c3d8b00b003d9ebf905c9so7086406wmb.5 for ; Thu, 12 Jan 2023 11:19:34 -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=fVn5IOLw5BZIPWuRa0Az7BeeqKdJycOl4yREX59G6Ig=; b=XgAgNQoPG8sDv2YNJKdmvG9llTGXv4YHfalKz5yXm1cIJRTu64YEIekZX+asuRBH6G KMfA0PBCfNtjWT6TWR44JMbztCDpsITYIT0+vQwX1awbh6jg0MuoP2HWjlPC2M2Oxtdl N6O6S2u8ZUDFQU/hANmrbDXkYjJcCx0MgytumTo4jKvWuTBwWm20kq6aBzdJu2TFgVKp 5omA1e2xT6x5K9/lEZSdN0FG5/2WuF6oDsday7awT5zVUoo3Ig393j8g/mqNyd5OTC2M buWNaW7vBZq3vSg8+PyDEVNidlhhkgWvVMsU1oye173f+0pb1NYS3f8LqX/Jk28xwka/ lHqA== X-Gm-Message-State: AFqh2kpgcpp3xcooADHnnCUi0u7fci9V3mGSvJIRpLBDByViXmpnHyBg FAl0oDAQo5FIWnOuSv1CyG8wbbMapgzokApFIv5Kcj46VvKXJtoKCNtNM1WASkxLF8AqVxAsCDE PMRKmGgBebKRhtuhGZBYbHct1PimQB2MwI4EQGNZBYLmS8wFjtKP689CUzudwFE+w2StNmAXKaw == X-Received: by 2002:a1c:6a16:0:b0:3c6:f732:bf6f with SMTP id f22-20020a1c6a16000000b003c6f732bf6fmr56149416wmc.13.1673551173371; Thu, 12 Jan 2023 11:19:33 -0800 (PST) X-Google-Smtp-Source: AMrXdXtp0aqXhK0FqfLT1p4AKpI/hxIaFUhqjfm+rTOzrEjkO2mgsrF/Wly24Npjs9plqmz5bVb01g== X-Received: by 2002:a1c:6a16:0:b0:3c6:f732:bf6f with SMTP id f22-20020a1c6a16000000b003c6f732bf6fmr56149407wmc.13.1673551173153; Thu, 12 Jan 2023 11:19:33 -0800 (PST) Received: from localhost (92.40.218.34.threembb.co.uk. [92.40.218.34]) by smtp.gmail.com with ESMTPSA id p5-20020a1c5445000000b003b4fe03c881sm1778895wmi.48.2023.01.12.11.19.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Jan 2023 11:19:32 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 3/3] gdb/tui: don't leak the known_window_types map Date: Thu, 12 Jan 2023 19:19:22 +0000 Message-Id: <52467d4e99bed2dfb4ef6fff7a7168c0e6f46f33.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 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Andrew Burgess via Gdb-patches From: Andrew Burgess Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" This commit finishes the task that was started in the previous commit. Now that all Python TUI window factories are correctly deleted when the Python interpreter is shut down, we no longer need to dynamically allocate the known_window_types map in tui-layout.c This commit changes known_window_types to a statically allocated data structure, removes the dynamic allocation from initialize_known_windows, and then replaces lots of '->' with '.' throughout this file. There should be no user visible changes after this commit. --- gdb/tui/tui-layout.c | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c index b895e00a80d..b65314ec30b 100644 --- a/gdb/tui/tui-layout.c +++ b/gdb/tui/tui-layout.c @@ -344,14 +344,9 @@ make_standard_window (const char *) return tui_win_list[V]; } -/* A map holding all the known window types, keyed by name. Note that - this is heap-allocated and "leaked" at gdb exit. This avoids - ordering issues with destroying elements in the map at shutdown. - In particular, destroying this map can occur after Python has been - shut down, causing crashes if any window destruction requires - running Python code. */ +/* A map holding all the known window types, keyed by name. */ -static std::unordered_map *known_window_types; +static std::unordered_map known_window_types; /* Helper function that returns a TUI window, given its name. */ @@ -362,8 +357,8 @@ tui_get_window_by_name (const std::string &name) if (name == window->name ()) return window; - auto iter = known_window_types->find (name); - if (iter == known_window_types->end ()) + auto iter = known_window_types.find (name); + if (iter == known_window_types.end ()) error (_("Unknown window type \"%s\""), name.c_str ()); tui_win_info *result = iter->second (name.c_str ()); @@ -377,20 +372,18 @@ tui_get_window_by_name (const std::string &name) static void initialize_known_windows () { - known_window_types = new std::unordered_map; - - known_window_types->emplace (SRC_NAME, + known_window_types.emplace (SRC_NAME, make_standard_window); - known_window_types->emplace (CMD_NAME, + known_window_types.emplace (CMD_NAME, make_standard_window); - known_window_types->emplace (DATA_NAME, + known_window_types.emplace (DATA_NAME, make_standard_window); - known_window_types->emplace (DISASSEM_NAME, + known_window_types.emplace (DISASSEM_NAME, make_standard_window); - known_window_types->emplace (STATUS_NAME, + known_window_types.emplace (STATUS_NAME, make_standard_window); } @@ -422,11 +415,11 @@ tui_register_window (const char *name, window_factory &&factory) this far then NAME must be a user defined window. Remove any existing factory and replace it with this new version. */ - auto iter = known_window_types->find (name); - if (iter != known_window_types->end ()) - known_window_types->erase (iter); + auto iter = known_window_types.find (name); + if (iter != known_window_types.end ()) + known_window_types.erase (iter); - known_window_types->emplace (std::move (name_copy), + known_window_types.emplace (std::move (name_copy), std::move (factory)); } @@ -1207,8 +1200,8 @@ initialize_layouts () static bool validate_window_name (const std::string &name) { - auto iter = known_window_types->find (name); - return iter != known_window_types->end (); + auto iter = known_window_types.find (name); + return iter != known_window_types.end (); } /* Implementation of the "tui new-layout" command. */