diff options
| -rw-r--r-- | gdb/target.c | 43 | ||||
| -rw-r--r-- | gdb/target.h | 4 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.python/py-connection-removed.exp | 92 |
3 files changed, 118 insertions, 21 deletions
diff --git a/gdb/target.c b/gdb/target.c index 1dd0f42..b3fd234 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -1182,23 +1182,28 @@ decref_target (target_ops *t) void target_stack::push (target_ops *t) { - t->incref (); + /* We must create a new reference first. It is possible that T is + already pushed on this target stack, in which case we will first + unpush it below, before re-pushing it. If we don't increment the + reference count now, then when we unpush it, we might end up deleting + T, which is not good. */ + auto ref = target_ops_ref::new_reference (t); strata stratum = t->stratum (); - if (stratum == process_stratum) - connection_list_add (as_process_stratum_target (t)); - /* If there's already a target at this stratum, remove it. */ - if (m_stack[stratum] != NULL) - unpush (m_stack[stratum]); + if (m_stack[stratum].get () != nullptr) + unpush (m_stack[stratum].get ()); /* Now add the new one. */ - m_stack[stratum] = t; + m_stack[stratum] = std::move (ref); if (m_top < stratum) m_top = stratum; + + if (stratum == process_stratum) + connection_list_add (as_process_stratum_target (t)); } /* See target.h. */ @@ -1223,19 +1228,19 @@ target_stack::unpush (target_ops *t) return false; } - /* Unchain the target. */ - m_stack[stratum] = NULL; - if (m_top == stratum) m_top = this->find_beneath (t)->stratum (); - /* Finally close the target, if there are no inferiors - referencing this target still. Note we do this after unchaining, - so any target method calls from within the target_close - implementation don't end up in T anymore. Do leave the target - open if we have are other inferiors referencing this target - still. */ - decref_target (t); + /* Move the target reference off the target stack, this sets the pointer + held in m_stack to nullptr, and places the reference in ref. When + ref goes out of scope its reference count will be decremented, which + might cause the target to close. + + We have to do it this way, and not just set the value in m_stack to + nullptr directly, because doing so would decrement the reference + count first, which might close the target, and closing the target + does a check that the target is not on any inferiors target_stack. */ + auto ref = std::move (m_stack[stratum]); return true; } @@ -3612,8 +3617,8 @@ target_stack::find_beneath (const target_ops *t) const { /* Look for a non-empty slot at stratum levels beneath T's. */ for (int stratum = t->stratum () - 1; stratum >= 0; --stratum) - if (m_stack[stratum] != NULL) - return m_stack[stratum]; + if (m_stack[stratum].get () != NULL) + return m_stack[stratum].get (); return NULL; } diff --git a/gdb/target.h b/gdb/target.h index 28aa927..b9888f8 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -1389,7 +1389,7 @@ public: { return at (t->stratum ()) == t; } /* Return the target at STRATUM. */ - target_ops *at (strata stratum) const { return m_stack[stratum]; } + target_ops *at (strata stratum) const { return m_stack[stratum].get (); } /* Return the target at the top of the stack. */ target_ops *top () const { return at (m_top); } @@ -1404,7 +1404,7 @@ private: /* The stack, represented as an array, with one slot per stratum. If no target is pushed at some stratum, the corresponding slot is null. */ - target_ops *m_stack[(int) debug_stratum + 1] {}; + std::array<target_ops_ref, (int) debug_stratum + 1> m_stack; }; /* Return the dummy target. */ diff --git a/gdb/testsuite/gdb.python/py-connection-removed.exp b/gdb/testsuite/gdb.python/py-connection-removed.exp new file mode 100644 index 0000000..6a0dbd1 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-connection-removed.exp @@ -0,0 +1,92 @@ +# Copyright (C) 2022 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 <http://www.gnu.org/licenses/>. + +# Check that the gdb.connect_removed event triggers when we expect it +# too. +# +# Checking this event has wider implications that simply some corner +# of the Python API working or not. The connection_removed event +# triggers when the reference count of a process_stratum_target +# reaches zero. If these events stop triggering when expected then +# GDB might be getting the reference counting on target_ops objects +# wrong. + +load_lib gdb-python.exp + +standard_testfile py-connection.c + +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } { + return -1 +} + +# Skip all tests if Python scripting is not enabled. +if { [skip_python_tests] } { continue } + +if ![runto_main] then { + return 0 +} + +# Register a callback that will trigger when a connection is removed +# (deleted) within GDB. +gdb_test_multiline "Add connection_removed event" \ + "python" "" \ + "def connection_removed_handler(event):" "" \ + " num = event.connection.num" "" \ + " type = event.connection.type" "" \ + " print('Connection %d (%s) removed' % (num, type))" "" \ + "gdb.events.connection_removed.connect (connection_removed_handler)" "" \ + "end" "" + +if { [target_info exists gdb_protocol] } { + if { [target_info gdb_protocol] == "extended-remote" } { + set connection_type "extended-remote" + } else { + set connection_type "remote" + } +} else { + set connection_type "native" +} + +# Add an inferior that shares a connection with inferior 1. +gdb_test "add-inferior" "Added inferior 2 on connection 1 \[^\r\n\]+" + +# Add an inferior with no connection. +gdb_test "add-inferior -no-connection" "Added inferior 3" + +# Kill the first inferior. If we are using the plain 'remote' protocol then +# it as this point that the remote target connection is removed. For the +# 'extended-remote' and 'native' targets the connection is removed later. +if { $connection_type == "remote" } { + gdb_test "with confirm off -- kill" \ + "Connection 1 \\(remote\\) removed\r\n.*" "kill inferior" +} else { + gdb_test "with confirm off -- kill" "" "kill inferior" +} + +# Switch to inferior 3 (the one with no connection). +gdb_test "inferior 3" + +# Remove inferior 1, not expecting anything interesting at this point. +gdb_test_no_output "remove-inferiors 1" + +# Now removed inferior 2. For the 'remote' target the connection has +# already been removed (see above), but for 'extended-remote' and 'native' +# targets, it is at this point that the connection is removed. +if { $connection_type == "remote" } { + gdb_test_no_output "remove-inferiors 2" +} else { + gdb_test "remove-inferiors 2" \ + "Connection 1 \\(${connection_type}\\) removed" +} |