From patchwork Wed Jan 25 13:56:09 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 63673 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 303C43858408 for ; Wed, 25 Jan 2023 13:57:09 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 303C43858408 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1674655029; bh=lx5+P/eNwhQENXKMfBNLHAc8WdKmmKU47Sjfte3shI0=; 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=oiaz61+lrb7Ne5TKszNOQnfYuoZS/EIt47SbO4BjfdRPbBeDBB2MK9T3pxiIRB2bO JRS3fE2Ow2wmkPyidI3rzM6jq81XIw+IMMO3FwCYlZmX7nOcyGs91LdH5PgyJtrZKD yMOfGh/oLWlPNpMqwutTBBpLsAG+jhXo+yxUhp6w= 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 864233858D28 for ; Wed, 25 Jan 2023 13:56:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 864233858D28 Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-90-PsbDq7DlPeS2BxOkL4Nr1w-1; Wed, 25 Jan 2023 08:56:18 -0500 X-MC-Unique: PsbDq7DlPeS2BxOkL4Nr1w-1 Received: by mail-qk1-f200.google.com with SMTP id bk3-20020a05620a1a0300b007092ce2a17eso9512487qkb.22 for ; Wed, 25 Jan 2023 05:56:18 -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=lx5+P/eNwhQENXKMfBNLHAc8WdKmmKU47Sjfte3shI0=; b=J0kYJV8Qs5w5MxgJS2st15nKVAVQvUKBLRFLJGARVApqlKeatNwEF3OlJQJEZ5YuAc ee600pPrEBme0YnA0b2H9QklA8hVZcCgvfUrRCgx2SticU9aiTdl+u4YWVP/BvAyr9rn YPJki5byz6U5tuh/b/KFAjSlxIcAY7M8KMB0e1JqhK9sSPGz64rQoPClhkAPMyQYT+pl TemxO71EyZ72h52X84lVR049wdaBrwNrhjrXNhXJrEmIF461Ni2lBzz6FzSYwBykVqlC ZdSOb0OEBjMTEcbVsF5UUUai+syQ6PAjIKsI0RUStnkf+j2OivSRpcOoIvaiGhnrhsn8 nYyw== X-Gm-Message-State: AFqh2kpJkKU7Z/xS5gttchgYOHehaepfPmAYTlqydYa03WZgxvmK3VAx XaO8o5+Mg3ScXNlQJu0zkk1srZUeM9qU3YowkK9R3BCUleVgXoGtsz8P6dkpQ0yYCo7+xgbhJFV rr1T4VGu6vf5DX71kw5ZTTLoYAlc3Kk3l5heSjVNk4Fh0xmGjMhaDNNYI/5UyYyI+wPOmUIpSOA == X-Received: by 2002:a0c:90ea:0:b0:51e:268a:e78b with SMTP id p97-20020a0c90ea000000b0051e268ae78bmr53703308qvp.33.1674654977450; Wed, 25 Jan 2023 05:56:17 -0800 (PST) X-Google-Smtp-Source: AMrXdXuiO5jY66HiXSecLR/B51BaIrp5cmjwNfeEUpU60VcQn+NKRpK24g+qUmg5ZlBqsqVNG62FaA== X-Received: by 2002:a0c:90ea:0:b0:51e:268a:e78b with SMTP id p97-20020a0c90ea000000b0051e268ae78bmr53703274qvp.33.1674654977068; Wed, 25 Jan 2023 05:56:17 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id u63-20020a372e42000000b0070209239b87sm3539518qkh.41.2023.01.25.05.56.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Jan 2023 05:56:16 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv2 1/3] gdb/python: allow Python TUI windows to be replaced Date: Wed, 25 Jan 2023 13:56:09 +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.8 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 | 73 +++++++++++++++++++ .../gdb.python/tui-window-factory.py | 48 ++++++++++++ gdb/tui/tui-layout.c | 8 ++ 3 files changed, 129 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..99f9fbb1bc4 --- /dev/null +++ b/gdb/testsuite/gdb.python/tui-window-factory.exp @@ -0,0 +1,73 @@ +# 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 + +require allow_tui_tests allow_python_tests + +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 Wed Jan 25 13:56:10 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 63672 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 2427C3858C74 for ; Wed, 25 Jan 2023 13:56:46 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2427C3858C74 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1674655006; bh=SaUFpB9/W3xhr/OgLrlCy5fTs/ugl6vIKqz33WV/8/A=; 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=kipHIYji0ffwexB6RlkvNNBD67LdhG5w+NDSVnChKDl827BRUoghuCNBlribNHsWF qxoAGPoh9Xf568+99/H2JNE4r/J7WKsdYiD2BE8hKb7l/7fXBtwVkr8pbdgg2mtnby Uqv0HKi10+UgwM0w8KSE8Cs7EC8B1cULi0axqYwY= 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 253DC3858C31 for ; Wed, 25 Jan 2023 13:56:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 253DC3858C31 Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-602-9JpU3RsxPZKATc0tGMi0sg-1; Wed, 25 Jan 2023 08:56:20 -0500 X-MC-Unique: 9JpU3RsxPZKATc0tGMi0sg-1 Received: by mail-qk1-f199.google.com with SMTP id h13-20020a05620a244d00b006fb713618b8so12930242qkn.0 for ; Wed, 25 Jan 2023 05:56:20 -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=SaUFpB9/W3xhr/OgLrlCy5fTs/ugl6vIKqz33WV/8/A=; b=2R08LGQk3ZsMRMt7WHENH2dgs+7gMTAbljxlpLOh6aGWxVxcryPCL7+S0SV0r7+17Z rhsBlLgUNPZWXIDwBVNYuxDoi0OtVM54l4VyEwKCVrAsSGNBadIyw1zmKZsF/K9MMayD FJXkPvejqP5ykRPDkAtDZfFKlV1/S/Z0VsIMHvS+oAjonrvjLF1JK3080ks9c7fHo4dH wNMB02oA7G11ZUsvrLJqngVQIJY7P2A81gaINk1L0Iicq8eCEBUHL9AxIg3U+aNLk9iF Sa5gLsId6OGDc/tP00oKha9JMmguzT0lj37ba8NPJgtgU0HJQfU8MHGBXXZKjKotsgbD QNTg== X-Gm-Message-State: AFqh2krFJ3gY4Ytw7A1N5AKlu5k3ikf8p/g4ADC1txGyRlL6iKAVKAGY p3asj/WBoJ6g5aA1zPJJhLmuPVB25AJhnlfmIZ9pKWWO0ZdF0cFbm/Z2NgKyaZj+UQ9MooYcP4S tjF2DOHljqzoKzWKe0oDjrGiHzTULz8UgwuE1PEncOJAxJFkRA+SQswWsOqDFNWJIltmZt+tzIA == X-Received: by 2002:a05:622a:4015:b0:3b0:7755:ab80 with SMTP id cf21-20020a05622a401500b003b07755ab80mr52374131qtb.67.1674654979825; Wed, 25 Jan 2023 05:56:19 -0800 (PST) X-Google-Smtp-Source: AMrXdXvM71gPNMJ4OfJmGUtuRRg7/0ioQBxpWNhiJB7/4jhPzFLKye9owDhN6//wKILx6RCpwvlEyQ== X-Received: by 2002:a05:622a:4015:b0:3b0:7755:ab80 with SMTP id cf21-20020a05622a401500b003b07755ab80mr52374090qtb.67.1674654979306; Wed, 25 Jan 2023 05:56:19 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id he32-20020a05622a602000b003a82562c90fsm3293763qtb.62.2023.01.25.05.56.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Jan 2023 05:56:18 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv2 2/3] gdb/python: deallocate tui window factories at Python shut down Date: Wed, 25 Jan 2023 13:56:10 +0000 Message-Id: <80daa36b6b15e84c88df642e50e01b494cce0a04.1674654912.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.8 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 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. --- 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 54623f445ed..ed466cc4511 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -1950,6 +1950,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 99f9fbb1bc4..3e898d01c7b 100644 --- a/gdb/testsuite/gdb.python/tui-window-factory.exp +++ b/gdb/testsuite/gdb.python/tui-window-factory.exp @@ -71,3 +71,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 Wed Jan 25 13:56:11 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 63674 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 03F6838582BE for ; Wed, 25 Jan 2023 13:57:14 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 03F6838582BE DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1674655034; 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=i4YlGCGE9gLFV01YoRJFjtYHsOmTaPeYbgyMN7fuInH7ZWDHOpOS2qppoxJarlG/J rjQdVB8GkDxBls3y9ZG7hUVHoNqeZDSsOosoHADCRlEajdUQLh2LvwKxLXNn00zrNS c3hYjU+y1ifbkwFh1En5B0IhyYTdlgpNCFKLhqoY= 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 321213858407 for ; Wed, 25 Jan 2023 13:56:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 321213858407 Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-392-8DZp_uDuPCOaRROibh9ieQ-1; Wed, 25 Jan 2023 08:56:22 -0500 X-MC-Unique: 8DZp_uDuPCOaRROibh9ieQ-1 Received: by mail-qk1-f200.google.com with SMTP id v7-20020a05620a0f0700b006faffce43b2so12829257qkl.9 for ; Wed, 25 Jan 2023 05:56:22 -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=hdLHP9nRZ6acr4ZhCH/dwMaqZuZ8r+lqOb6TNuD5sKAouT4jAUTmpp423C6g7jOBjD 1RB6ELNLDmjGW3Y4Ikf7uSfD5R00rGopve7iVGTO8PwEbDB3ixaOK6MfFSAKALO8b3wg O10ffdb93sOk/mBW/uuok2K//zqh3P9wzlDw882pl+BFiedGdk6QxnNqotiau2ndqwpE +rMGNyFaCfokKz4EWVTpy6bMXyoo+ZNZioegiAUkypit7mXWzeVvAbb96Tqy8v3ZePVQ KGWgUcL/dqffRL4N8bCY7f/dAhuw9+viPW77y0uJTdZtjkCafh9X+3ifmupO6sydbaz7 4PSw== X-Gm-Message-State: AFqh2kpOXF0Ein77kqbcpVhmycK3Slq59xaJvOAP59kNWk+bS99fZpmZ +J2y5ejAGMR9w8grugM2ZRRJf3heUdWN8WVFVxShIZ5koo7vWEEkv2ol8dQ8RFM/DHvbYoAvh0i RGoWtzVn87Du++5IoU61r/odTKKto68SvQZjzBzujPgnoOOAeHnSrz1dpujW2wDQzabIXDrwX4Q == X-Received: by 2002:ac8:4cc7:0:b0:3ab:2a7f:83e4 with SMTP id l7-20020ac84cc7000000b003ab2a7f83e4mr41907815qtv.35.1674654981842; Wed, 25 Jan 2023 05:56:21 -0800 (PST) X-Google-Smtp-Source: AMrXdXvsSokmsF+dOXtA1SbQyTYzhB4iN3NlBIRXpdhuBGugAMtfoKYGkuWY+lO5pXfmPanhHtr1kw== X-Received: by 2002:ac8:4cc7:0:b0:3ab:2a7f:83e4 with SMTP id l7-20020ac84cc7000000b003ab2a7f83e4mr41907793qtv.35.1674654981523; Wed, 25 Jan 2023 05:56:21 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id f14-20020a37ad0e000000b0070495934152sm3534712qkm.48.2023.01.25.05.56.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Jan 2023 05:56:21 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv2 3/3] gdb/tui: don't leak the known_window_types map Date: Wed, 25 Jan 2023 13:56:11 +0000 Message-Id: <39fcef41919b141f6f1cd48d6a4b6f4d3b22ded3.1674654912.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.8 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. */