Browse Source

Fix x11 display server crash when deleting popup window when unfocused

On FocusOut events, the window could be destroyed while propagating
WINDOW_EVENT_FOCUS_OUT event, which causes the WindowData to be
invalidated, and still used for calls to XUnsetICFocus.

This change moves calls to XUnsetICFocus, and also XSetICFocus in
FocusIn events, before propagating the change of focus event to the
engine, to be safe in any case.

Also setting xic member to nullptr after all calls to XDestroyIC to keep
things clean and consistent.

Fixes #42645
PouleyKetchoupp 4 years ago
parent
commit
48a0d44e67
1 changed files with 21 additions and 16 deletions
  1. 21 16
      platform/linuxbsd/display_server_x11.cpp

+ 21 - 16
platform/linuxbsd/display_server_x11.cpp

@@ -737,6 +737,7 @@ void DisplayServerX11::delete_sub_window(WindowID p_id) {
 	XDestroyWindow(x11_display, wd.x11_window);
 	XDestroyWindow(x11_display, wd.x11_window);
 	if (wd.xic) {
 	if (wd.xic) {
 		XDestroyIC(wd.xic);
 		XDestroyIC(wd.xic);
+		wd.xic = nullptr;
 	}
 	}
 
 
 	windows.erase(p_id);
 	windows.erase(p_id);
@@ -2784,6 +2785,13 @@ void DisplayServerX11::process_events() {
 
 
 				wd.focused = true;
 				wd.focused = true;
 
 
+				if (wd.xic) {
+					// Block events polling while changing input focus
+					// because it triggers some event polling internally.
+					MutexLock mutex_lock(events_mutex);
+					XSetICFocus(wd.xic);
+				}
+
 				// Keep track of focus order for overlapping windows.
 				// Keep track of focus order for overlapping windows.
 				static unsigned int focus_order = 0;
 				static unsigned int focus_order = 0;
 				wd.focus_order = ++focus_order;
 				wd.focus_order = ++focus_order;
@@ -2812,12 +2820,6 @@ void DisplayServerX11::process_events() {
 					XIGrabDevice(x11_display, xi.touch_devices[i], x11_window, CurrentTime, None, XIGrabModeAsync, XIGrabModeAsync, False, &xi.touch_event_mask);
 					XIGrabDevice(x11_display, xi.touch_devices[i], x11_window, CurrentTime, None, XIGrabModeAsync, XIGrabModeAsync, False, &xi.touch_event_mask);
 				}*/
 				}*/
 #endif
 #endif
-				if (wd.xic) {
-					// Block events polling while changing input focus
-					// because it triggers some event polling internally.
-					MutexLock mutex_lock(events_mutex);
-					XSetICFocus(wd.xic);
-				}
 
 
 				if (!app_focused) {
 				if (!app_focused) {
 					if (OS::get_singleton()->get_main_loop()) {
 					if (OS::get_singleton()->get_main_loop()) {
@@ -2834,6 +2836,13 @@ void DisplayServerX11::process_events() {
 
 
 				wd.focused = false;
 				wd.focused = false;
 
 
+				if (wd.xic) {
+					// Block events polling while changing input focus
+					// because it triggers some event polling internally.
+					MutexLock mutex_lock(events_mutex);
+					XUnsetICFocus(wd.xic);
+				}
+
 				Input::get_singleton()->release_pressed_events();
 				Input::get_singleton()->release_pressed_events();
 				_send_window_event(wd, WINDOW_EVENT_FOCUS_OUT);
 				_send_window_event(wd, WINDOW_EVENT_FOCUS_OUT);
 
 
@@ -2864,12 +2873,6 @@ void DisplayServerX11::process_events() {
 				}
 				}
 				xi.state.clear();
 				xi.state.clear();
 #endif
 #endif
-				if (wd.xic) {
-					// Block events polling while changing input focus
-					// because it triggers some event polling internally.
-					MutexLock mutex_lock(events_mutex);
-					XUnsetICFocus(wd.xic);
-				}
 			} break;
 			} break;
 
 
 			case ConfigureNotify: {
 			case ConfigureNotify: {
@@ -4044,11 +4047,13 @@ DisplayServerX11::~DisplayServerX11() {
 		}
 		}
 #endif
 #endif
 
 
-		if (E->get().xic) {
-			XDestroyIC(E->get().xic);
+		WindowData &wd = E->get();
+		if (wd.xic) {
+			XDestroyIC(wd.xic);
+			wd.xic = nullptr;
 		}
 		}
-		XUnmapWindow(x11_display, E->get().x11_window);
-		XDestroyWindow(x11_display, E->get().x11_window);
+		XUnmapWindow(x11_display, wd.x11_window);
+		XDestroyWindow(x11_display, wd.x11_window);
 	}
 	}
 
 
 	//destroy drivers
 	//destroy drivers