wayland: Notes for future readers on use of get_window_with_id and window->terminated. (#59)

Author: ivucica

Headers/wayland/WaylandServer.h

@@ -169,6 +169,13 @@ struct window
   CairoSurface		       *wcs;
 };
 
+/* get_window_with_id returns the known window with the passed ID, or NULL.
+ *
+ * NULL is returned when the passed window is not known; for example, the
+ * window has been '->terminated' and was removed from the window_list already,
+ * but something has referred to its ID. It is the responsibility of the caller
+ * to handle the case where NULL is used.
+ */
 struct window *get_window_with_id(WaylandConfig *wlconfig, int winid);
 
 @interface WaylandServer : GSDisplayServer

Source/wayland/WaylandServer+Xdgshell.m

@@ -40,6 +40,22 @@
 
   if (window->terminated == YES)
     {
+      // 'struct window' is defined in Headers/wayland/WaylandServer.
+      //
+      // window->terminated should only be true after
+      // -[WaylandServer(WindowOps) termwindow:(int)win] sets this to true
+      // after invoking -[WaylandServer destroyWindowShell:window] and
+      // using wl_list_remove(&window->link);.
+      //
+      // We do not expect 'window' to be referenced again, since
+      // -destroyWindowShell: invokes wl_display_dispatch_pending(...) and
+      // wl_display_flush(...);. On the off chance it does, first, we may want
+      // to patch every single invocation of get_window_with_id() that may
+      // currently be ignoring the case where NULL may be returned, and
+      // possibly crashing for that reason.
+      //
+      // But, a free here should on its own be fine as long as everyone
+      // passes around the window ID and does not store a ptr to window itself.
       NSDebugLog(@"deleting window win=%d", window->window_id);
       free(window);
       return;

Source/wayland/WaylandServer.m

@@ -137,6 +137,8 @@ static void handle_global_remove(void *data, struct wl_registry *registry,
     handle_global, handle_global_remove};
 
 struct window *get_window_with_id(WaylandConfig *wlconfig, int winid) {
+  /* This can return NULL. A relevant note has been added to the docstring
+   * in the header. Callers should be handling this. */
   struct window *window;
 
   wl_list_for_each(window, &wlconfig->window_list, link) {