fix: resolve nested IPC socket collisions for instantwmctl#241
fix: resolve nested IPC socket collisions for instantwmctl#241paperbenni merged 1 commit intomainfrom
Conversation
Update `src/ipc/mod.rs` to dynamically find an available socket path
(e.g. `/tmp/instantwm-{uid}-{i}.sock`) rather than blindly removing
and overwriting the default socket. This allows nested instances
of the window manager to maintain their own IPC socket.
Update `src/bin/ctl/ipc.rs` to prioritize the `INSTANTWM_SOCKET`
environment variable when connecting, ensuring `instantwmctl`
connects to the appropriate nested instance when spawned from it.
Co-authored-by: paperbenni <15818888+paperbenni@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Reviewer's GuideAdjusts IPC socket handling so nested instantwm instances use distinct, auto-discovered socket paths and makes instantwmctl prefer the INSTANTWM_SOCKET environment variable for connecting. Sequence diagram for nested instantwm IPC server socket bindingsequenceDiagram
actor User
participant InstantwmParent as Instantwm_parent
participant InstantwmNested as Instantwm_nested
participant FS as Filesystem
participant Env as Environment
User->>InstantwmParent: start instantwm
InstantwmParent->>InstantwmParent: IpcServer.bind()
InstantwmParent->>InstantwmParent: get_available_socket_path()
loop find first free socket for parent
InstantwmParent->>FS: check /tmp/instantwm-{uid}.sock exists
alt does not exist
InstantwmParent->>InstantwmParent: use base path
else exists
InstantwmParent->>FS: connect UnixStream to path
alt connect fails (dead socket)
InstantwmParent->>FS: remove_file(path)
InstantwmParent->>InstantwmParent: use cleaned path
else connect ok (active server)
InstantwmParent->>InstantwmParent: i += 1
end
end
end
InstantwmParent->>FS: bind UnixListener at chosen path
InstantwmParent->>Env: set_var INSTANTWM_SOCKET=chosen_path
User->>InstantwmNested: start instantwm --backend nested
InstantwmNested->>InstantwmNested: IpcServer.bind()
InstantwmNested->>InstantwmNested: get_available_socket_path()
loop find next free socket for nested
InstantwmNested->>FS: check /tmp/instantwm-{uid}.sock or -i.sock exists
alt socket belongs to parent (active)
InstantwmNested->>FS: connect UnixStream to path
InstantwmNested->>InstantwmNested: i += 1
else dead or missing
InstantwmNested->>FS: remove dead socket if needed
InstantwmNested->>InstantwmNested: use this path
end
end
InstantwmNested->>FS: bind UnixListener at nested path
InstantwmNested->>Env: set_var INSTANTWM_SOCKET=nested_path
Sequence diagram for instantwmctl IPC socket resolution with environment overridesequenceDiagram
actor User
participant Env as Environment
participant Instantwmctl as Instantwmctl
participant FS as Filesystem
User->>Instantwmctl: run instantwmctl command
Instantwmctl->>Instantwmctl: get_default_socket()
Instantwmctl->>Env: read INSTANTWM_SOCKET
alt INSTANTWM_SOCKET is set
Env-->>Instantwmctl: socket_path_from_env
Instantwmctl->>Instantwmctl: use socket_path_from_env
else INSTANTWM_SOCKET not set
Env-->>Instantwmctl: error
Instantwmctl->>Instantwmctl: format /tmp/instantwm-{uid}.sock
end
Instantwmctl->>FS: connect UnixStream to chosen socket path
FS-->>Instantwmctl: connection result
Instantwmctl-->>User: IPC response or error
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe changes modify socket path handling: the control tool now reads the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This pull request resolves an issue where running
instantwm --backend nestedwithin an existinginstantwmsession causedinstantwmctlto misbehave by clobbering the parent's IPC socket.src/ipc/mod.rs): Instead of hardcoding the socket path to/tmp/instantwm-{uid}.sockand blindly overwriting it, the IPC server now checks for active sockets and intelligently creates sequentially numbered socket paths (/tmp/instantwm-{uid}-1.sock, etc.) when running nested. Dead sockets are still gracefully cleaned up.src/bin/ctl/ipc.rs):instantwmctlnow respects theINSTANTWM_SOCKETenvironment variable (which is exported by the compositor to its nested clients).This guarantees proper separation of IPC namespaces between host and nested compositors.
PR created automatically by Jules for task 2559026827372285636 started by @paperbenni
Summary by Sourcery
Ensure IPC sockets are uniquely allocated to avoid collisions between host and nested instantwm instances.
Bug Fixes:
Summary by CodeRabbit
New Features
INSTANTWM_SOCKETenvironment variable to customize socket location.Bug Fixes