Ensure the debugger controller is not created in the UI action enable…

… callbacks. Fix Vector35/binaryninja#502
1ikeadragon · Sep 5, 2023 · e152ccb · e152ccb
1 parent 037f403
commit e152ccb
Showing 1 changed file with 21 additions and 0 deletions.
diff --git a/ui/ui.cpp b/ui/ui.cpp
@@ -235,6 +235,13 @@ void GlobalDebuggerUI::SetupMenu(UIContext* context)
 	auto notConnected = [=](const UIActionContext& ctxt) {
 		if (!ctxt.binaryView)
 			return false;
+		// TODO: these two calls should be combined into something like GetControllerIfExists
+		// The reason why we must avoid creating a new debugger controller here is because these enable callbacks
+		// are called in unpredictable order compared to the destroy of the controller when we close a tab.
+		// There is a chance that we first destroy the controller, and they quickly recreate it, causing a memory
+		// leak for the underlying binary view.
+		if (!DebuggerController::ControllerExists(ctxt.binaryView))
+			return false;
 		auto controller = DebuggerController::GetController(ctxt.binaryView);
 		if (!controller)
 			return false;
@@ -245,6 +252,8 @@ void GlobalDebuggerUI::SetupMenu(UIContext* context)
 	auto connected = [=](const UIActionContext& ctxt) {
 		if (!ctxt.binaryView)
 			return false;
+		if (!DebuggerController::ControllerExists(ctxt.binaryView))
+			return false;
 		auto controller = DebuggerController::GetController(ctxt.binaryView);
 		if (!controller)
 			return false;
@@ -255,6 +264,8 @@ void GlobalDebuggerUI::SetupMenu(UIContext* context)
 	auto connectedAndStopped = [=](const UIActionContext& ctxt) {
 		if (!ctxt.binaryView)
 			return false;
+		if (!DebuggerController::ControllerExists(ctxt.binaryView))
+			return false;
 		auto controller = DebuggerController::GetController(ctxt.binaryView);
 		if (!controller)
 			return false;
@@ -265,6 +276,8 @@ void GlobalDebuggerUI::SetupMenu(UIContext* context)
 	auto connectedAndRunning = [=](const UIActionContext& ctxt) {
 		if (!ctxt.binaryView)
 			return false;
+		if (!DebuggerController::ControllerExists(ctxt.binaryView))
+			return false;
 		auto controller = DebuggerController::GetController(ctxt.binaryView);
 		if (!controller)
 			return false;
@@ -275,6 +288,8 @@ void GlobalDebuggerUI::SetupMenu(UIContext* context)
 	auto connectedToDebugServer = [=](const UIActionContext& ctxt) {
 		if (!ctxt.binaryView)
 			return false;
+		if (!DebuggerController::ControllerExists(ctxt.binaryView))
+			return false;
 		auto controller = DebuggerController::GetController(ctxt.binaryView);
 		if (!controller)
 			return false;
@@ -285,6 +300,8 @@ void GlobalDebuggerUI::SetupMenu(UIContext* context)
 	auto notConnectedToDebugServer = [=](const UIActionContext& ctxt) {
 		if (!ctxt.binaryView)
 			return false;
+		if (!DebuggerController::ControllerExists(ctxt.binaryView))
+			return false;
 		auto controller = DebuggerController::GetController(ctxt.binaryView);
 		if (!controller)
 			return false;
@@ -1349,6 +1366,8 @@ static void RunToHereCallback(BinaryView* view, uint64_t addr)
 
 static bool ConnectedAndStopped(BinaryView* view, uint64_t addr)
 {
+	if (!DebuggerController::ControllerExists(view))
+		return false;
 	auto controller = DebuggerController::GetController(view);
 	if (!controller)
 		return false;
@@ -1358,6 +1377,8 @@ static bool ConnectedAndStopped(BinaryView* view, uint64_t addr)
 
 static bool ConnectedAndRunning(BinaryView* view, uint64_t addr)
 {
+	if (!DebuggerController::ControllerExists(view))
+		return false;
 	auto controller = DebuggerController::GetController(view);
 	if (!controller)
 		return false;