godotengine
/
godot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #59908 from bruvzg/fix_popup_close_race 

Fix a possible race condition on popup close, that might cause multiple deletions of the same list item.
This commit is contained in:
Rémi Verschelde 2022-04-06 08:35:56 +02:00 committed by GitHub
parent e4f0fc50f7 daa42e0e50
commit b79721fede
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 67 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
platform/linuxbsd/display_server_x11.cpp
View File

@ -3208,20 +3208,24 @@ Rect2i DisplayServerX11::window_get_popup_safe_rect(WindowID p_window) const {
} }
void DisplayServerX11::popup_open(WindowID p_window) { void DisplayServerX11::popup_open(WindowID p_window) {
_THREAD_SAFE_METHOD_
WindowData &wd = windows[p_window]; WindowData &wd = windows[p_window];
if (wd.is_popup) { if (wd.is_popup) {
// Close all popups, up to current popup parent, or every popup if new window is not transient. // Find current popup parent, or root popup if new window is not transient.
List<WindowID>::Element *C = nullptr;
List<WindowID>::Element *E = popup_list.back(); List<WindowID>::Element *E = popup_list.back();
while (E) { while (E) {
if (wd.transient_parent != E->get() || wd.transient_parent == INVALID_WINDOW_ID) { if (wd.transient_parent != E->get() || wd.transient_parent == INVALID_WINDOW_ID) {
_send_window_event(windows[E->get()], DisplayServerX11::WINDOW_EVENT_CLOSE_REQUEST); C = E;
List<WindowID>::Element *F = E->prev(); E = E->prev();
popup_list.erase(E);
E = F;
} else { } else {
break; break;
} }
} }
if (C) {
_send_window_event(windows[C->get()], DisplayServerX11::WINDOW_EVENT_CLOSE_REQUEST);
}
time_since_popup = OS::get_singleton()->get_ticks_msec(); time_since_popup = OS::get_singleton()->get_ticks_msec();
popup_list.push_back(p_window); popup_list.push_back(p_window);
@ -3229,16 +3233,22 @@ void DisplayServerX11::popup_open(WindowID p_window) {
} }
void DisplayServerX11::popup_close(WindowID p_window) { void DisplayServerX11::popup_close(WindowID p_window) {
_THREAD_SAFE_METHOD_
List<WindowID>::Element *E = popup_list.find(p_window); List<WindowID>::Element *E = popup_list.find(p_window);
while (E) { while (E) {
_send_window_event(windows[E->get()], DisplayServerX11::WINDOW_EVENT_CLOSE_REQUEST);
List<WindowID>::Element *F = E->next(); List<WindowID>::Element *F = E->next();
WindowID win_id = E->get();
popup_list.erase(E); popup_list.erase(E);
_send_window_event(windows[win_id], DisplayServerX11::WINDOW_EVENT_CLOSE_REQUEST);
E = F; E = F;
} }
} }
void DisplayServerX11::mouse_process_popups() { void DisplayServerX11::mouse_process_popups() {
_THREAD_SAFE_METHOD_
if (popup_list.is_empty()) { if (popup_list.is_empty()) {
return; return;
} }
@ -3259,7 +3269,9 @@ void DisplayServerX11::mouse_process_popups() {
Vector2i pos = Vector2i(root_attrs.x + root_x, root_attrs.y + root_y); Vector2i pos = Vector2i(root_attrs.x + root_x, root_attrs.y + root_y);
if ((pos != last_mouse_monitor_pos) || (mask != last_mouse_monitor_mask)) { if ((pos != last_mouse_monitor_pos) || (mask != last_mouse_monitor_mask)) {
if (((mask & Button1Mask) || (mask & Button2Mask) || (mask & Button3Mask) || (mask & Button4Mask) || (mask & Button5Mask))) { if (((mask & Button1Mask) || (mask & Button2Mask) || (mask & Button3Mask) || (mask & Button4Mask) || (mask & Button5Mask))) {
List<WindowID>::Element *C = nullptr;
List<WindowID>::Element *E = popup_list.back(); List<WindowID>::Element *E = popup_list.back();
// Find top popup to close.
while (E) { while (E) {
// Popup window area. // Popup window area.
Rect2i win_rect = Rect2i(window_get_position(E->get()), window_get_size(E->get())); Rect2i win_rect = Rect2i(window_get_position(E->get()), window_get_size(E->get()));
@ -3270,12 +3282,13 @@ void DisplayServerX11::mouse_process_popups() {
} else if (safe_rect != Rect2i() && safe_rect.has_point(pos)) { } else if (safe_rect != Rect2i() && safe_rect.has_point(pos)) {
break; break;
} else { } else {
_send_window_event(windows[E->get()], DisplayServerX11::WINDOW_EVENT_CLOSE_REQUEST); C = E;
List<WindowID>::Element *F = E->prev(); E = E->prev();
popup_list.erase(E);
E = F;
} }
} }
if (C) {
_send_window_event(windows[C->get()], DisplayServerX11::WINDOW_EVENT_CLOSE_REQUEST);
}
} }
} }
last_mouse_monitor_mask = mask; last_mouse_monitor_mask = mask;

36
platform/osx/display_server_osx.mm
View File

@ -3000,21 +3000,25 @@ Rect2i DisplayServerOSX::window_get_popup_safe_rect(WindowID p_window) const {
} }
void DisplayServerOSX::popup_open(WindowID p_window) { void DisplayServerOSX::popup_open(WindowID p_window) {
_THREAD_SAFE_METHOD_
WindowData &wd = windows[p_window]; WindowData &wd = windows[p_window];
if (wd.is_popup) { if (wd.is_popup) {
bool was_empty = popup_list.is_empty(); bool was_empty = popup_list.is_empty();
// Close all popups, up to current popup parent, or every popup if new window is not transient. // Find current popup parent, or root popup if new window is not transient.
List<WindowID>::Element *C = nullptr;
List<WindowID>::Element *E = popup_list.back(); List<WindowID>::Element *E = popup_list.back();
while (E) { while (E) {
if (wd.transient_parent != E->get() || wd.transient_parent == INVALID_WINDOW_ID) { if (wd.transient_parent != E->get() || wd.transient_parent == INVALID_WINDOW_ID) {
send_window_event(windows[E->get()], DisplayServerOSX::WINDOW_EVENT_CLOSE_REQUEST); C = E;
List<WindowID>::Element *F = E->prev(); E = E->prev();
popup_list.erase(E);
E = F;
} else { } else {
break; break;
} }
} }
if (C) {
send_window_event(windows[C->get()], DisplayServerOSX::WINDOW_EVENT_CLOSE_REQUEST);
}
if (was_empty && popup_list.is_empty()) { if (was_empty && popup_list.is_empty()) {
// Inform OS that popup was opened, to close other native popups. // Inform OS that popup was opened, to close other native popups.
@ -3026,12 +3030,16 @@ void DisplayServerOSX::popup_open(WindowID p_window) {
} }
void DisplayServerOSX::popup_close(WindowID p_window) { void DisplayServerOSX::popup_close(WindowID p_window) {
_THREAD_SAFE_METHOD_
bool was_empty = popup_list.is_empty(); bool was_empty = popup_list.is_empty();
List<WindowID>::Element *E = popup_list.find(p_window); List<WindowID>::Element *E = popup_list.find(p_window);
while (E) { while (E) {
send_window_event(windows[E->get()], DisplayServerOSX::WINDOW_EVENT_CLOSE_REQUEST);
List<WindowID>::Element *F = E->next(); List<WindowID>::Element *F = E->next();
WindowID win_id = E->get();
popup_list.erase(E); popup_list.erase(E);
send_window_event(windows[win_id], DisplayServerOSX::WINDOW_EVENT_CLOSE_REQUEST);
E = F; E = F;
} }
if (!was_empty && popup_list.is_empty()) { if (!was_empty && popup_list.is_empty()) {
@ -3047,11 +3055,8 @@ void DisplayServerOSX::mouse_process_popups(bool p_close) {
if (p_close) { if (p_close) {
// Close all popups. // Close all popups.
List<WindowID>::Element *E = popup_list.front(); List<WindowID>::Element *E = popup_list.front();
while (E) { if (E) {
send_window_event(windows[E->get()], DisplayServerOSX::WINDOW_EVENT_CLOSE_REQUEST); send_window_event(windows[E->get()], DisplayServerOSX::WINDOW_EVENT_CLOSE_REQUEST);
List<WindowID>::Element *F = E->next();
popup_list.erase(E);
E = F;
} }
if (!was_empty) { if (!was_empty) {
// Inform OS that all popups are closed. // Inform OS that all popups are closed.
@ -3064,7 +3069,9 @@ void DisplayServerOSX::mouse_process_popups(bool p_close) {
} }
Point2i pos = mouse_get_position(); Point2i pos = mouse_get_position();
List<WindowID>::Element *C = nullptr;
List<WindowID>::Element *E = popup_list.back(); List<WindowID>::Element *E = popup_list.back();
// Find top popup to close.
while (E) { while (E) {
// Popup window area. // Popup window area.
Rect2i win_rect = Rect2i(window_get_position(E->get()), window_get_size(E->get())); Rect2i win_rect = Rect2i(window_get_position(E->get()), window_get_size(E->get()));
@ -3075,12 +3082,13 @@ void DisplayServerOSX::mouse_process_popups(bool p_close) {
} else if (safe_rect != Rect2i() && safe_rect.has_point(pos)) { } else if (safe_rect != Rect2i() && safe_rect.has_point(pos)) {
break; break;
} else { } else {
send_window_event(windows[E->get()], DisplayServerOSX::WINDOW_EVENT_CLOSE_REQUEST); C = E;
List<WindowID>::Element *F = E->prev(); E = E->prev();
popup_list.erase(E);
E = F;
} }
} }
if (C) {
send_window_event(windows[C->get()], DisplayServerOSX::WINDOW_EVENT_CLOSE_REQUEST);
}
if (!was_empty && popup_list.is_empty()) { if (!was_empty && popup_list.is_empty()) {
// Inform OS that all popups are closed. // Inform OS that all popups are closed.
[[NSDistributedNotificationCenter defaultCenter] postNotificationName:@"com.apple.HIToolbox.endMenuTrackingNotification" object:@"org.godotengine.godot.popup_window"]; [[NSDistributedNotificationCenter defaultCenter] postNotificationName:@"com.apple.HIToolbox.endMenuTrackingNotification" object:@"org.godotengine.godot.popup_window"];

33
platform/windows/display_server_windows.cpp
View File

@ -2081,20 +2081,24 @@ Rect2i DisplayServerWindows::window_get_popup_safe_rect(WindowID p_window) const
} }
void DisplayServerWindows::popup_open(WindowID p_window) { void DisplayServerWindows::popup_open(WindowID p_window) {
_THREAD_SAFE_METHOD_
WindowData &wd = windows[p_window]; WindowData &wd = windows[p_window];
if (wd.is_popup) { if (wd.is_popup) {
// Close all popups, up to current popup parent, or every popup if new window is not transient. // Find current popup parent, or root popup if new window is not transient.
List<WindowID>::Element *C = nullptr;
List<WindowID>::Element *E = popup_list.back(); List<WindowID>::Element *E = popup_list.back();
while (E) { while (E) {
if (wd.transient_parent != E->get() || wd.transient_parent == INVALID_WINDOW_ID) { if (wd.transient_parent != E->get() || wd.transient_parent == INVALID_WINDOW_ID) {
_send_window_event(windows[E->get()], DisplayServerWindows::WINDOW_EVENT_CLOSE_REQUEST); C = E;
List<WindowID>::Element *F = E->prev(); E = E->prev();
popup_list.erase(E);
E = F;
} else { } else {
break; break;
} }
} }
if (C) {
_send_window_event(windows[C->get()], DisplayServerWindows::WINDOW_EVENT_CLOSE_REQUEST);
}
time_since_popup = OS::get_singleton()->get_ticks_msec(); time_since_popup = OS::get_singleton()->get_ticks_msec();
popup_list.push_back(p_window); popup_list.push_back(p_window);
@ -2102,17 +2106,22 @@ void DisplayServerWindows::popup_open(WindowID p_window) {
} }
void DisplayServerWindows::popup_close(WindowID p_window) { void DisplayServerWindows::popup_close(WindowID p_window) {
_THREAD_SAFE_METHOD_
List<WindowID>::Element *E = popup_list.find(p_window); List<WindowID>::Element *E = popup_list.find(p_window);
while (E) { while (E) {
_send_window_event(windows[E->get()], DisplayServerWindows::WINDOW_EVENT_CLOSE_REQUEST);
List<WindowID>::Element *F = E->next(); List<WindowID>::Element *F = E->next();
WindowID win_id = E->get();
popup_list.erase(E); popup_list.erase(E);
_send_window_event(windows[win_id], DisplayServerWindows::WINDOW_EVENT_CLOSE_REQUEST);
E = F; E = F;
} }
} }
LRESULT DisplayServerWindows::MouseProc(int code, WPARAM wParam, LPARAM lParam) { LRESULT DisplayServerWindows::MouseProc(int code, WPARAM wParam, LPARAM lParam) {
_THREAD_SAFE_METHOD_ _THREAD_SAFE_METHOD_
uint64_t delta = OS::get_singleton()->get_ticks_msec() - time_since_popup; uint64_t delta = OS::get_singleton()->get_ticks_msec() - time_since_popup;
if (delta > 250) { if (delta > 250) {
switch (wParam) { switch (wParam) {
@ -2123,7 +2132,9 @@ LRESULT DisplayServerWindows::MouseProc(int code, WPARAM wParam, LPARAM lParam)
case WM_MBUTTONDOWN: { case WM_MBUTTONDOWN: {
MOUSEHOOKSTRUCT *ms = (MOUSEHOOKSTRUCT *)lParam; MOUSEHOOKSTRUCT *ms = (MOUSEHOOKSTRUCT *)lParam;
Point2i pos = Point2i(ms->pt.x, ms->pt.y); Point2i pos = Point2i(ms->pt.x, ms->pt.y);
List<WindowID>::Element *C = nullptr;
List<WindowID>::Element *E = popup_list.back(); List<WindowID>::Element *E = popup_list.back();
// Find top popup to close.
while (E) { while (E) {
// Popup window area. // Popup window area.
Rect2i win_rect = Rect2i(window_get_position(E->get()), window_get_size(E->get())); Rect2i win_rect = Rect2i(window_get_position(E->get()), window_get_size(E->get()));
@ -2134,13 +2145,13 @@ LRESULT DisplayServerWindows::MouseProc(int code, WPARAM wParam, LPARAM lParam)
} else if (safe_rect != Rect2i() && safe_rect.has_point(pos)) { } else if (safe_rect != Rect2i() && safe_rect.has_point(pos)) {
break; break;
} else { } else {
_send_window_event(windows[E->get()], DisplayServerWindows::WINDOW_EVENT_CLOSE_REQUEST); C = E;
List<WindowID>::Element *F = E->prev(); E = E->prev();
popup_list.erase(E);
E = F;
} }
} }
if (C) {
_send_window_event(windows[C->get()], DisplayServerWindows::WINDOW_EVENT_CLOSE_REQUEST);
}
} break; } break;
} }
} }