From patchwork Fri Jan 27 16:34:03 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 63798 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 072DB385B537 for ; Fri, 27 Jan 2023 16:34:42 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 072DB385B537 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1674837282; bh=H8tbuNcicnZIv88O1nozCkz1phWN3lvs+Q1h6pSe8LU=; 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=pq6leugwBLonbE9C3P/XhiiCUqJeghOTBKtRKAQcB5r83roJFvF5S6zz7j6uIzT9c 4hluUagqjVehWgutDV8EV+c0tpF2R0GA/NDP2KN9drMcZz1j3Yqiw41bm9Xie3BuLL ZdpX75coaYr4UQq01OYb+JbMGXz5qHVeG3IzcCtg= 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 6E8243858C2F for ; Fri, 27 Jan 2023 16:34:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6E8243858C2F Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-148-Yo-doLvwMx-sHw8J106sbQ-1; Fri, 27 Jan 2023 11:34:11 -0500 X-MC-Unique: Yo-doLvwMx-sHw8J106sbQ-1 Received: by mail-qt1-f200.google.com with SMTP id j14-20020ac874ce000000b003b6917d0731so2338159qtr.4 for ; Fri, 27 Jan 2023 08:34:11 -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=H8tbuNcicnZIv88O1nozCkz1phWN3lvs+Q1h6pSe8LU=; b=RNJ8Hpzs0JSY4T+xoUwZjB0FdmE+poQyjU3ptBUd+jF5CeJ7YEfmpgfpNP/TK3wIyF jsHtu3ilPcnTfbUydb8IGMV9Cpsf8zS49FHNmmo2U8Ag5O8L4nY85x0QzfDZjW5JoEhB elSeY/zg73WRNardfIVFhPBMB4HIn2+q5y1t6lNk6//uiWbQ8+geyJwlkmR3NWfdLbre IJRqqyoDMXqwybH8DGjcYHVtvJAlFAHdY89cpc55eJznR51NtxEO700UlDrowW6pXLTJ T5nr9IGLNJIcO9E5UI9Bs1VVNc503EF+6LgXL+kZmG2NffsAN9PmigGVDimNy9kxDcgt 1Xqg== X-Gm-Message-State: AFqh2kqIRDEQa4oMFSoQC1U3SmPaB6Ln4gpsS0TVvwksv/Qu6gC6ps/y ZvlFe8CX8moUndKWE2QJkjxKmwoIWAnllvhxycjkYkpzzictIfoiB1J26Atg6jILfQAHBpSrUS9 QbepQyNojaDMm+SpBmVukW04SloXyB18UsR2d46nnq6t+GUN0AF7PR6ibYfXE83SXKCEy43pmtw == X-Received: by 2002:ac8:12c2:0:b0:3a9:90d9:58be with SMTP id b2-20020ac812c2000000b003a990d958bemr58730752qtj.56.1674837250416; Fri, 27 Jan 2023 08:34:10 -0800 (PST) X-Google-Smtp-Source: AMrXdXtqzNfALgxedW+rdyYRcAqCFlmSAX5zTjFIv1nE4TY8TCStfSOfDnmjrT6v0Gq5ye/V2YEW1Q== X-Received: by 2002:ac8:12c2:0:b0:3a9:90d9:58be with SMTP id b2-20020ac812c2000000b003a990d958bemr58730728qtj.56.1674837250064; Fri, 27 Jan 2023 08:34:10 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id s39-20020a05622a1aa700b003a7e38055c9sm2943217qtc.63.2023.01.27.08.34.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 Jan 2023 08:34:09 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv3 1/3] gdb/python: allow Python TUI windows to be replaced Date: Fri, 27 Jan 2023 16:34:03 +0000 Message-Id: <7fefb92241aed87ddf8c6e61183fd629c622d819.1674837139.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.7 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 ecdcd4884a7..ba7ec89973e 100644 --- a/gdb/tui/tui-layout.c +++ b/gdb/tui/tui-layout.c @@ -427,6 +427,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 Fri Jan 27 16:34:04 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 63799 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 0E5F138555A0 for ; Fri, 27 Jan 2023 16:34:45 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0E5F138555A0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1674837285; 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=VX7dx+IDcYk4/4BAF1olinKKQGN4ePs6zDCvy8o3bIM+Da+YEbYzXH+WVmUXpFeJ+ 9gj7aJyMi56eF3bv+oMLUAfuOiSR0jACC1KsAqB6WktAwpcO/9yJQ/ZOi9XJmRQZtB GvQq39eHqUaKng8SPlxkEEgUoVJ9IqEPZ0NNBPkY= 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 39B843857BB2 for ; Fri, 27 Jan 2023 16:34:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 39B843857BB2 Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-120-_krUrqiyP2agElQbo8schw-1; Fri, 27 Jan 2023 11:34:13 -0500 X-MC-Unique: _krUrqiyP2agElQbo8schw-1 Received: by mail-qk1-f197.google.com with SMTP id x12-20020a05620a258c00b007051ae500a2so3283665qko.15 for ; Fri, 27 Jan 2023 08:34:12 -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=g0jMuGV95QfWFiQdCwnEvAjkkuACoX+ItozhthA4SI5jKJpYEzLMylj58NVqafd8xP qX1r4KNt514KIWnXdBZVMvkhOR7vhzjYjBC3Bz3EZruhD7EWOW/yd6XpUk8uY2Oz+uo6 g7XgXz5wOBrIVkdDcfEjW0XlxkKgBflN6Heg2rP5K4VR/0fxNobQr3mUI6+I+Xkm2ZJS ElYz3ajCeoa1xov81XZP9MG4xysv6nEMTHehwG45ZlIhu8YUA/PBw6mI1FPu9aqglNSh zwmhNq5akVTLpDN7Ljsty7rDfVYtaYmdJQXb8D7Wmx+W09sVuQK8NRWomVxPlY1e9VXj AS6Q== X-Gm-Message-State: AFqh2kqzpUscx9ymezkuIqU280JJZkzKLS/8P+Ek6hgnPsSwfypAPzIC c4fwoNA3+EIsWh23d3Z6/wtjMdoLPpaO3Cysf7sv207Fr5RWQo5hT8dlv0afUfQNeGBKlNqMw3V 3qZz3oqj9E7SOnpUZZErPvi2BdIqL9yEc0G9GEWoLqRWaTl7MKjU4yACt6+F5kjT+/1cmygJn2w == X-Received: by 2002:ac8:5b90:0:b0:3b1:5df1:9167 with SMTP id a16-20020ac85b90000000b003b15df19167mr80694406qta.19.1674837252327; Fri, 27 Jan 2023 08:34:12 -0800 (PST) X-Google-Smtp-Source: AMrXdXvjjQ/+tCbOuzjJUYTH7RTNDt0iky9jLTDm7pwi7BEjZViXXgkKIUZ/Rg5TPjEair+sL/5UMg== X-Received: by 2002:ac8:5b90:0:b0:3b1:5df1:9167 with SMTP id a16-20020ac85b90000000b003b15df19167mr80694365qta.19.1674837251967; Fri, 27 Jan 2023 08:34:11 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id jt9-20020a05622aa00900b003b80fdaa14dsm2930112qtb.73.2023.01.27.08.34.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 Jan 2023 08:34:11 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv3 2/3] gdb/python: deallocate tui window factories at Python shut down Date: Fri, 27 Jan 2023 16:34:04 +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, 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 Fri Jan 27 16:34:05 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 63800 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 991193857B98 for ; Fri, 27 Jan 2023 16:35:11 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 991193857B98 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1674837311; bh=tw0xzkhaRlnTYrlN44thru6LpnSRhdlTRwON8IxO9yo=; 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=TF2/HR+ErG5FhXWbdHEI4Jw0inS5aoi7zxpS/BFpXovNzGvAGO0K+6yWRnXayqoq4 WiMT1ee/8Eri5vmCAk/Z6kxE34tvpyYQM9igM1VmKp3S+GVl7eef1YJXjDo+iA+m2Z aljF+nIHXYloM+XnBkmfn3DMF2JM3pORv7KV7uM8= 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 A21B63857835 for ; Fri, 27 Jan 2023 16:34:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A21B63857835 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-205-YnTiArsLOqW2Zx79IXbXPg-1; Fri, 27 Jan 2023 11:34:14 -0500 X-MC-Unique: YnTiArsLOqW2Zx79IXbXPg-1 Received: by mail-qk1-f200.google.com with SMTP id q21-20020a05620a0d9500b0070572ccdbf9so3299113qkl.10 for ; Fri, 27 Jan 2023 08:34:14 -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=tw0xzkhaRlnTYrlN44thru6LpnSRhdlTRwON8IxO9yo=; b=j79c7ypt3WzUGp4s7q9alrGs8zx1U6Hggt/GaARV9lNyQN9qpbxXD2TTpSApHB8LfR z+bV1BZnHxVkX5LreOWJV4rRe/sqatltZP3FYnk5aNrfmWsYvdBpIFKeff89iSE5I7yI jaRvcPteuKCjhSPBH+O6i9908EXD8hIRc34vvuBK9kmEbEsTdwrWZJglxRKHMdgVKwLW Ob6IJ94oNdoyev4LrfbFfvu74YescJ0SGA33qeOOuLatNPjOZ8eMojs83JBhybH0snUF oO7tHob4FC3A92qkMYxcBPgjbGMwfqakSEEnd5FZjcS7H6dj5hagtbtcmAMU6zkm8H6n 6IYw== X-Gm-Message-State: AFqh2kppajvsyqi1b/Rvyi3wEEgH3otDi0RZ8T+2Egi0mRTj1AC/KFZu fEodP6YEjenNOp8CzPTwYpm6+t0QQuzAyD/2EKE+c2vNMt7gDdxMBw0JW1khxGDCMLKRgwBeiQ1 jr7eJBl1mKG+BgVQUBrSpk856C5NZ7D+CQ7IM3fWDTH1ZuA/QzM4unDCYIrMk6Psi0ST0ssSAZA == X-Received: by 2002:ac8:5c86:0:b0:3a7:e9a2:4f4a with SMTP id r6-20020ac85c86000000b003a7e9a24f4amr38286989qta.8.1674837254233; Fri, 27 Jan 2023 08:34:14 -0800 (PST) X-Google-Smtp-Source: AMrXdXsU5Y1yakhj7t+TIzCjXJJbIpMmrMjoMjyRSePkS9191BQqjkQoasx3DmJKtQQrTvVhhO++2A== X-Received: by 2002:ac8:5c86:0:b0:3a7:e9a2:4f4a with SMTP id r6-20020ac85c86000000b003a7e9a24f4amr38286949qta.8.1674837253822; Fri, 27 Jan 2023 08:34:13 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id s15-20020ac85ccf000000b003b691385327sm1483514qta.6.2023.01.27.08.34.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 27 Jan 2023 08:34:13 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv3 3/3] gdb/tui: don't leak the known_window_types map Date: Fri, 27 Jan 2023 16:34:05 +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, 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 | 41 +++++++++++++++++------------------------ 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c index ba7ec89973e..bb8356c5544 100644 --- a/gdb/tui/tui-layout.c +++ b/gdb/tui/tui-layout.c @@ -343,22 +343,17 @@ 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 window_types_map *known_window_types; +static window_types_map known_window_types; /* See tui-layout.h. */ known_window_names_range all_known_window_names () { - auto begin = known_window_names_iterator (known_window_types->begin ()); - auto end = known_window_names_iterator (known_window_types->end ()); + auto begin = known_window_names_iterator (known_window_types.begin ()); + auto end = known_window_names_iterator (known_window_types.end ()); return known_window_names_range (begin, end); } @@ -371,8 +366,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 ()); @@ -386,20 +381,18 @@ tui_get_window_by_name (const std::string &name) static void initialize_known_windows () { - known_window_types = new window_types_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); } @@ -431,11 +424,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)); } @@ -1216,8 +1209,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. */