Don't assume that exwm--connection is non-nil

[nbarrientos]

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 #<xcb:SetInputFocus ...

even worse, not even giving the chance to the warn call in exwm-init's error handler to actually inform the user about the actual problem ("[XELB] Connection timeout", for instance).

[medranocalvo]

Dear @nbarrientos, let me start saying that I really appreciate that you took the time to investigate this edge case, delve in EXWM sources and submit this tiny improvement to EXWM's robustness.

Reviewing the patch I wondered whether that's the only exwm-*--exit function using exwm--connection: it's not (exwm-workspace--exit, ...). I think that the logic error is before: if we haven't been able to build an exwm--connection then no exwh-*--init could run, so there should be no need to run the exwm-*--exit functions either. Another perspective: the xcb:connect error with the fixed exwm-input--exit will fail in a similar manner at exwm-workspace--exit (I think?).

So, xcb:connect errors should be handled in exwm--exit directly, in my opinion, in that the module's *--exit functions should not be invoked.

Now, if xcb:connect succeeds, EXWM starts cleanly but eventually the connection is broken, then will we want to cleanup EXWM. That is, we want our exwm-*—exit functions to clean up without error despite the connection being unavailable. For this situation is when your fix is needed. In addition to your fix we’ll have to fix other modules’ use of exwm—connection in their exwm-*—exit functions.

I think that next steps would be:

• Patch exwm--exit to not run exwm-*--exit when there's no exwm--connection. This should fix the issue you describe above.
• Modify this patch to check for (slot-value exwm--connection 'connected) instead. Perhaps we should introduce a macro (exwm-when-connected forms)?
• Fix everything else ;-).

[medranocalvo]

I adjusted the your commit message (http://git.savannah.gnu.org/cgit/emacs.git/tree/CONTRIBUTE#Commit+messages) and added a commit implementing what I described above. Please, have a look and test if you can. I've evaluated (xcb:disconnect exwm--connection) in the *scratch* buffer... it led to a SEGFAULT.

[nbarrientos]

Hi @medranocalvo

Glad to see you around! Many thanks for taking a look.

Indeed when I looked at the problem I thought too that it might be nicer to re-do a bit how the "there's no valid connection to the X server when cleaning up" situation is handled, as you're suggesting. I decided though to go for the smallest patch possible addressing the exact issue I was facing, hoping it'd more likely to get it merged and also because my knowledge of EXWM's codebase is rather limited. I'm happy to see that you've spent some time on improving the situation, thanks.

I haven't looked in detail at your patch as currently I don't have much time but your train of thoughts and the diff make sense.

[medranocalvo]

Merged into master. Thank you!

[medranocalvo]

This fix is included in the recent 0.28 release. Thank you!