From cff02333e282c692aa685d57cc2b7c32419fffe2 Mon Sep 17 00:00:00 2001 From: Nacho Barrientos Date: Sun, 7 May 2023 12:45:31 +0200 Subject: [PATCH 1/2] Don't assume that exwm--connection is non-nil `exwm-input--exit` could be called (via `exwm-exit`) from `exwm-init` in case of error when initialising EXWM. It could happen that the bit that failed when exwm-init is executed was the call to `xcb:connect`, hence `exwm--connection` would be nil when errors are handled (and `exwm-exit` is called). Without this patch, in the case above, the user will see a crash as there's no method allowing a nil XCB connection object: Debugger entered--Lisp error: (cl-no-applicable-method xcb:-+request nil # Date: Fri, 9 Jun 2023 00:00:00 +0000 Subject: [PATCH 2/2] Observe connection status on deinitialization * exwm-workspace.el (exwm-workspace--remove-frame-as-workspace): Add optional argument quit. * exwm-background.el (exwm-background--exit): * exwm-input.el (exwm-input--exit): * exwm-manage.el (exwm-manage--unmanage-window): * exwm-systemtray.el (exwm-systemtray--exit): * exwm-workspace.el (exwm-workspace--exit-minibuffer-frame) (exwm-workspace--exit): * exwm-xim.el (exwm-xim--exit): Observe connection status when deinitializing in order to support deinitializing when the connection breaks. --- exwm-background.el | 6 ++-- exwm-input.el | 2 +- exwm-manage.el | 9 ++++-- exwm-systemtray.el | 27 ++++++++-------- exwm-workspace.el | 81 +++++++++++++++++++++++++--------------------- exwm-xim.el | 6 ++-- exwm.el | 10 +++--- 7 files changed, 77 insertions(+), 64 deletions(-) diff --git a/exwm-background.el b/exwm-background.el index e7a0360..f3e0d89 100644 --- a/exwm-background.el +++ b/exwm-background.el @@ -172,19 +172,17 @@ replace it.") (defun exwm-background--init () "Initialize background module." (exwm--log) - (add-hook 'enable-theme-functions 'exwm-background--update) (add-hook 'disable-theme-functions 'exwm-background--update) - (exwm-background--update)) (defun exwm-background--exit () "Uninitialize the background module." (exwm--log) - (remove-hook 'enable-theme-functions 'exwm-background--update) (remove-hook 'disable-theme-functions 'exwm-background--update) - (when exwm-background--connection + (when (and exwm-background--connection + (slot-value exwm-background--connection 'connected)) (xcb:disconnect exwm-background--connection)) (setq exwm-background--pixmap nil exwm-background--connection nil diff --git a/exwm-input.el b/exwm-input.el index 2c65116..3f9a305 100644 --- a/exwm-input.el +++ b/exwm-input.el @@ -1215,7 +1215,7 @@ One use is to access the keymap bound to KEYS (as prefix keys) in char-mode." (when exwm-input--update-focus-timer (cancel-timer exwm-input--update-focus-timer)) ;; Make input focus working even without a WM. - (when exwm--connection + (when (slot-value exwm--connection 'connected) (xcb:+request exwm--connection (make-instance 'xcb:SetInputFocus :revert-to xcb:InputFocus:PointerRoot diff --git a/exwm-manage.el b/exwm-manage.el index c3d47f7..1adf66d 100644 --- a/exwm-manage.el +++ b/exwm-manage.el @@ -430,7 +430,9 @@ manager is shutting down." (exwm-workspace--update-workareas) (dolist (f exwm-workspace--list) (exwm-workspace--set-fullscreen f))) - (when (buffer-live-p buffer) + (when (and (buffer-live-p buffer) + ;; Invoked from `exwm-manage--exit' upon disconnection. + (slot-value exwm--connection 'connected)) (with-current-buffer buffer ;; Unmap the X window. (xcb:+request exwm--connection @@ -512,8 +514,11 @@ manager is shutting down." (defun exwm-manage--kill-buffer-query-function () "Run in `kill-buffer-query-functions'." - (exwm--log "id=#x%x; buffer=%s" exwm--id (current-buffer)) + (exwm--log "id=#x%x; buffer=%s" (or exwm--id 0) (current-buffer)) (catch 'return + (when (or (not exwm--connection) + (not (slot-value exwm--connection 'connected))) + (throw 'return t)) (when (or (not exwm--id) (xcb:+request-checked+request-check exwm--connection (make-instance 'xcb:ChangeWindowAttributes diff --git a/exwm-systemtray.el b/exwm-systemtray.el index 0f19986..51d5213 100644 --- a/exwm-systemtray.el +++ b/exwm-systemtray.el @@ -652,19 +652,20 @@ indicate how to support actual transparency." "Exit the systemtray module." (exwm--log) (when exwm-systemtray--connection - ;; Hide & reparent out the embedder before disconnection to prevent - ;; embedded icons from being reparented to an Emacs frame (which is the - ;; parent of the embedder). - (xcb:+request exwm-systemtray--connection - (make-instance 'xcb:UnmapWindow - :window exwm-systemtray--embedder-window)) - (xcb:+request exwm-systemtray--connection - (make-instance 'xcb:ReparentWindow - :window exwm-systemtray--embedder-window - :parent exwm--root - :x 0 - :y 0)) - (xcb:disconnect exwm-systemtray--connection) + (when (slot-value exwm-systemtray--connection 'connected) + ;; Hide & reparent out the embedder before disconnection to prevent + ;; embedded icons from being reparented to an Emacs frame (which is the + ;; parent of the embedder). + (xcb:+request exwm-systemtray--connection + (make-instance 'xcb:UnmapWindow + :window exwm-systemtray--embedder-window)) + (xcb:+request exwm-systemtray--connection + (make-instance 'xcb:ReparentWindow + :window exwm-systemtray--embedder-window + :parent exwm--root + :x 0 + :y 0)) + (xcb:disconnect exwm-systemtray--connection)) (setq exwm-systemtray--connection nil exwm-systemtray--list nil exwm-systemtray--selection-owner-window nil diff --git a/exwm-workspace.el b/exwm-workspace.el index 06217a7..67e3523 100644 --- a/exwm-workspace.el +++ b/exwm-workspace.el @@ -1389,32 +1389,34 @@ Return nil if FRAME is the only workspace." (unless (eq frame nextw) nextw))) -(defun exwm-workspace--remove-frame-as-workspace (frame) +(defun exwm-workspace--remove-frame-as-workspace (frame &optional quit) "Stop treating frame FRAME as a workspace." ;; TODO: restore all frame parameters (e.g. exwm-workspace, buffer-predicate, ;; etc) (exwm--log "Removing frame `%s' as workspace" frame) - (let* ((next-frame (exwm-workspace--get-next-workspace frame)) - (following-frames (cdr (memq frame exwm-workspace--list)))) - ;; Need to remove the workspace from the list for the correct calculation of - ;; indexes below. - (setq exwm-workspace--list (delete frame exwm-workspace--list)) - (unless next-frame - ;; The user managed to delete the last workspace, so create a new one. - (exwm--log "Last workspace deleted; create a new one") - (let ((exwm-workspace--create-silently t)) - (setq next-frame (make-frame)))) - (dolist (pair exwm--id-buffer-alist) - (let ((other-frame (buffer-local-value 'exwm--frame (cdr pair)))) - ;; Move X windows to next-frame. - (when (eq other-frame frame) - (exwm-workspace-move-window next-frame (car pair))) - ;; Update the _NET_WM_DESKTOP property of each following X window. - (when (memq other-frame following-frames) - (exwm-workspace--set-desktop (car pair))))) - ;; If the current workspace is deleted, switch to next one. - (when (eq frame exwm-workspace--current) - (exwm-workspace-switch next-frame))) + (unless quit + (let* ((next-frame (exwm-workspace--get-next-workspace frame)) + (following-frames (cdr (memq frame exwm-workspace--list)))) + ;; Need to remove the workspace from the list for the correct calculation of + ;; indexes below. + (setq exwm-workspace--list (delete frame exwm-workspace--list)) + ;; Move the windows to the next workspace and switch to it. + (unless next-frame + ;; The user managed to delete the last workspace, so create a new one. + (exwm--log "Last workspace deleted; create a new one") + (let ((exwm-workspace--create-silently t)) + (setq next-frame (make-frame)))) + (dolist (pair exwm--id-buffer-alist) + (let ((other-frame (buffer-local-value 'exwm--frame (cdr pair)))) + ;; Move X windows to next-frame. + (when (eq other-frame frame) + (exwm-workspace-move-window next-frame (car pair))) + ;; Update the _NET_WM_DESKTOP property of each following X window. + (when (memq other-frame following-frames) + (exwm-workspace--set-desktop (car pair))))) + ;; If the current workspace is deleted, switch to next one. + (when (eq frame exwm-workspace--current) + (exwm-workspace-switch next-frame)))) ;; Reparent out the frame. (let ((outer-id (frame-parameter frame 'exwm-outer-id))) (xcb:+request exwm--connection @@ -1448,8 +1450,9 @@ Return nil if FRAME is the only workspace." ;; Update EWMH properties. (exwm-workspace--update-ewmh-props) ;; Update switch history. - (setq exwm-workspace--switch-history-outdated t) - (run-hooks 'exwm-workspace-list-change-hook)) + (unless quit + (setq exwm-workspace--switch-history-outdated t) + (run-hooks 'exwm-workspace-list-change-hook))) (defun exwm-workspace--on-delete-frame (frame) "Hook run upon `delete-frame' that tears down FRAME's configuration as a workspace." @@ -1623,7 +1626,9 @@ applied to all subsequently created X frames." (setq default-minibuffer-frame nil) (when (frame-live-p exwm-workspace--minibuffer) ; might be already dead (let ((id (frame-parameter exwm-workspace--minibuffer 'exwm-outer-id))) - (when (and exwm-workspace--minibuffer id) + (when (and exwm-workspace--minibuffer id + ;; Invoked from `exwm-manage--exit' upon disconnection. + (slot-value exwm--connection 'connected)) (xcb:+request exwm--connection (make-instance 'xcb:ReparentWindow :window id @@ -1708,19 +1713,21 @@ applied to all subsequently created X frames." #'exwm-workspace--on-echo-area-clear)) ;; Hide & reparent out all frames (save-set can't be used here since ;; X windows will be re-mapped). - (setq exwm-workspace--current nil) - (dolist (i exwm-workspace--list) - (when (frame-live-p i) ; might be already dead - (exwm-workspace--remove-frame-as-workspace i) - (modify-frame-parameters i '((exwm-selected-window . nil) - (exwm-urgency . nil) - (exwm-outer-id . nil) - (exwm-id . nil) - (exwm-container . nil) - ;; (internal-border-width . nil) ; integerp - (fullscreen . nil) - (buffer-predicate . nil))))) + (when (slot-value exwm--connection 'connected) + (dolist (i exwm-workspace--list) + (when (frame-live-p i) ; might be already dead + (exwm-workspace--remove-frame-as-workspace i 'quit) + (modify-frame-parameters i '((exwm-selected-window . nil) + (exwm-urgency . nil) + (exwm-outer-id . nil) + (exwm-id . nil) + (exwm-container . nil) + ;; (internal-border-width . nil) ; integerp + (fullscreen . nil) + (buffer-predicate . nil)))))) ;; Don't let dead frames linger. + (setq exwm-workspace--current nil) + (setq exwm-workspace-current-index 0) (setq exwm-workspace--list nil)) (defun exwm-workspace--post-init () diff --git a/exwm-xim.el b/exwm-xim.el index 9589648..ac530f3 100644 --- a/exwm-xim.el +++ b/exwm-xim.el @@ -754,10 +754,12 @@ Such event would be received when the client window is destroyed." ;; Close IMS communication connections. (mapc (lambda (i) (when (vectorp i) - (xcb:disconnect (elt i 0)))) + (when (slot-value (elt i 0) 'connected) + (xcb:disconnect (elt i 0))))) exwm-xim--server-client-plist) ;; Close the IMS connection. - (unless exwm-xim--conn + (unless (and exwm-xim--conn + (slot-value exwm-xim--conn 'connected)) (cl-return-from exwm-xim--exit)) ;; Remove exwm-xim from XIM_SERVERS. (let ((reply (xcb:+request-unchecked+reply exwm-xim--conn diff --git a/exwm.el b/exwm.el index 345f76b..31fe684 100644 --- a/exwm.el +++ b/exwm.el @@ -907,12 +907,12 @@ manager. If t, replace it, if nil, abort and ask the user if `ask'." (run-hooks 'exwm-exit-hook) (setq confirm-kill-emacs nil) ;; Exit modules. - (exwm-input--exit) - (exwm-manage--exit) - (exwm-workspace--exit) - (exwm-floating--exit) - (exwm-layout--exit) (when exwm--connection + (exwm-input--exit) + (exwm-manage--exit) + (exwm-workspace--exit) + (exwm-floating--exit) + (exwm-layout--exit) (xcb:flush exwm--connection) (xcb:disconnect exwm--connection)) (setq exwm--connection nil)