Fix UDP terminal handling and X11/libevent cleanup

[etiaro]

Fixes several lifecycle and resource-management issues:

• Prevents repeated X11 event registration and hotkey grabs during rendering.
• Cleans up X11, Cairo, shared-memory, and libevent resources safely on shutdown.
• Ensures renderer cleanup occurs before the shared libevent base is released.
• Treats an absent UART as -1 and skips terminal configuration in UDP mode, preserving the caller’s terminal settings.

Assisted by: OpenAI Codex 5.6

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Fix UDP terminal handling and X11/libevent shutdown ownership

🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Prevent repeated X11/libevent event registration and repeated hotkey grabs during render flush.
• Make renderer shutdown idempotent and safely release Cairo/X11/XShm resources.
• Avoid clobbering caller terminal settings by skipping termios in UDP mode.

Diagram

graph TD
  A["msposd.c main loop"] --> B["libevent base"] --> C["Render_gs renderer"] --> D["X11 FD + hotkeys"]
  E["UDP/UART input"] --> A
  A --> F["CloseMSP() cleanup"] --> G["event_base_free + global shutdown"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Move X11/libevent hook into explicit renderer init/shutdown API

• ➕ Avoids doing setup work in FlushDrawing() entirely
• ➕ Makes ownership clearer: renderer_init(base) registers, renderer_close() unregisters
• ➕ Easier to ensure key grabs are paired with XUngrabKey and tested
• ➖ Requires a broader API change and call-site updates
• ➖ Slightly larger refactor than the current targeted fix

2. Centralize all event registration in msposd.c (renderer provides only callbacks)

• ➕ Single place owns all libevent objects, simplifying shutdown ordering
• ➕ Renderer becomes purely drawing + X11 resource management
• ➖ More coupling between msposd and renderer internals (X11 connection number, callbacks)
• ➖ Bigger change footprint for a bugfix PR

Recommendation: Current approach is a good targeted fix for repeated event creation and shutdown ordering without broad refactors. If X11 integration grows, consider an explicit renderer_init(base)/renderer_close() interface (and optionally XUngrabKey) to keep event-loop wiring out of per-frame rendering and make ownership/contracts unambiguous.

Files changed (2) +69 / -20

Bug fix (2) +69 / -20

msposd.cSkip termios in UDP mode and fix shutdown ordering/guards +14/-6

Skip termios in UDP mode and fix shutdown ordering/guards

• Initializes serial_fd to -1 and treats “no UART” as absent so UDP mode doesn't apply raw termios settings to stdin. Tightens UART-present checks (>= 0) for libevent/bufferevent setup and MSP polling. Initializes/frees SIGTERM event safely and calls CloseMSP() before freeing the event base and global libevent state.

msposd.c

Render_gs.cPrevent repeated X11 event registration; make renderer cleanup safe/idempotent +55/-14

Prevent repeated X11 event registration; make renderer cleanup safe/idempotent

• Stores the X11 libevent event in the global guard so FlushDrawing() doesn't recreate events or re-grab hotkeys each frame, and checks event_new() success before event_add(). Reworks Close() to NULL-check and release Cairo surfaces/contexts, unregister/free the X11 event, and safely detach/free XShm and X11 resources; removes freeing the shared event_base (renderer no longer owns it).

osd/util/Render_gs.c

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. Ungated XGrabKey on failure 🐞 Bug ☼ Reliability

Description

In FlushDrawing(), XGrabKey() executes even if event_new() fails, leaving x11_event NULL; since
FlushDrawing() is called repeatedly, this can cause repeated hotkey-grab attempts every frame
without ever registering the X11 fd with libevent. This can create noisy X11 errors/unnecessary work
and still leaves keyboard event handling disabled.

Code

osd/util/Render_gs.c[R521-524]

+        if (x11_event)
+            event_add(x11_event, NULL);

        XGrabKey(display, XKeysymToKeycode(display, XK_Up), Mod1Mask, RootWindow, True,

Evidence

The added code only guards event_add() behind if (x11_event) but does not guard XGrabKey().
Since FlushDrawing() is called each frame, a persistent allocation/registration failure will
repeatedly execute the grab calls while x11_event stays NULL.

osd/util/Render_gs.c[510-531]
osd.c[2831-2839]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`FlushDrawing()` unconditionally calls `XGrabKey()` after attempting `event_new()`. If `event_new()` (or `event_add()`) fails, `x11_event` remains `NULL`, and the next `FlushDrawing()` call will retry and repeatedly attempt key grabs every frame.

### Issue Context
This code runs in the render loop; `FlushDrawing()` is invoked repeatedly during normal operation.

### Fix Focus Areas
- osd/util/Render_gs.c[516-530]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[qodo-free-for-open-source-projects]

1. Ungated xgrabkey on failure 🐞 Bug ☼ Reliability

In FlushDrawing(), XGrabKey() executes even if event_new() fails, leaving x11_event NULL; since
FlushDrawing() is called repeatedly, this can cause repeated hotkey-grab attempts every frame
without ever registering the X11 fd with libevent. This can create noisy X11 errors/unnecessary work
and still leaves keyboard event handling disabled.

Agent Prompt

### Issue description
`FlushDrawing()` unconditionally calls `XGrabKey()` after attempting `event_new()`. If `event_new()` (or `event_add()`) fails, `x11_event` remains `NULL`, and the next `FlushDrawing()` call will retry and repeatedly attempt key grabs every frame.

### Issue Context
This code runs in the render loop; `FlushDrawing()` is invoked repeatedly during normal operation.

### Fix Focus Areas
- osd/util/Render_gs.c[516-530]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools