Skip to content

refactor(fspy): use shared memory IPC on Windows#239

Merged
branchseer merged 6 commits intomainfrom
10-21-refactor_fspy_use_shared_memory_ipc_on_windows
Oct 23, 2025
Merged

refactor(fspy): use shared memory IPC on Windows#239
branchseer merged 6 commits intomainfrom
10-21-refactor_fspy_use_shared_memory_ipc_on_windows

Conversation

@branchseer
Copy link
Copy Markdown
Member

@branchseer branchseer commented Oct 21, 2025

TL;DR

Replaced Windows pipe-based IPC with shared memory for file system access tracking, unifying the approach across platforms.

What changed?

  • Replaced Windows named pipe IPC with shared memory for tracking file system accesses
  • Created a new ipc.rs module with common code for both Windows and Unix platforms
  • Extracted OwnedReceiverLockGuard from Unix implementation to be reused on Windows
  • Removed dependency on dashmap which was used for the previous Windows implementation
  • Added write_encoded method to ShmWriter to directly encode values into shared memory
  • Updated Windows payload structure to use ChannelConf instead of pipe handles

Why make this change?

The previous implementation used different IPC mechanisms on Windows (named pipes) and Unix (shared memory). This change unifies the approach across platforms, making the code more maintainable and consistent. Shared memory is more efficient for passing large amounts of data between processes, which is important for tracking file system accesses in complex applications. The unified approach also simplifies the codebase by removing platform-specific code paths and dependencies.

Copy link
Copy Markdown
Member Author

branchseer commented Oct 21, 2025

@branchseer branchseer force-pushed the 10-21-refactor_fspy_use_shared_memory_ipc_on_windows branch 2 times, most recently from 0a72112 to 204b6cd Compare October 21, 2025 15:39
@branchseer branchseer marked this pull request as ready for review October 21, 2025 15:47
@branchseer branchseer requested a review from Copilot October 21, 2025 15:48
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the Windows IPC mechanism for file system access tracking to use shared memory instead of named pipes, aligning it with the Unix implementation. The change consolidates platform-specific code paths, removes the dashmap dependency, and introduces a unified OwnedReceiverLockGuard structure for managing IPC receivers across platforms.

Key Changes:

  • Replaced Windows named pipe IPC with shared memory channels
  • Unified IPC handling code in a new ipc.rs module shared across platforms
  • Added write_encoded method to ShmWriter for direct encoding into shared memory

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crates/fspy_shared/src/windows/mod.rs Updated Payload structure to use ChannelConf instead of pipe handle
crates/fspy_shared/src/ipc/channel/shm_io.rs Added write_encoded method and renamed test helper function
crates/fspy_shared/src/ipc/channel/mod.rs Added Windows-specific configuration for raw shared memory
crates/fspy_shared/Cargo.toml Added thiserror dependency
crates/fspy_preload_windows/src/windows/mod.rs Removed cleanup call during DLL detach
crates/fspy_preload_windows/src/windows/client.rs Replaced pipe-based IPC with shared memory sender
crates/fspy_preload_windows/Cargo.toml Removed dashmap dependency
crates/fspy/src/windows/mod.rs Replaced pipe creation and reading with shared memory channel
crates/fspy/src/unix/mod.rs Moved OwnedReceiverLockGuard to shared module
crates/fspy/src/lib.rs Added new ipc module
crates/fspy/src/ipc.rs Created shared IPC module with OwnedReceiverLockGuard

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread crates/fspy_shared/src/ipc/channel/shm_io.rs Outdated
Comment thread crates/fspy_preload_windows/src/windows/client.rs
Comment thread crates/fspy_preload_windows/src/windows/client.rs
Comment thread crates/fspy/src/ipc.rs
@Brooooooklyn
Copy link
Copy Markdown
Member

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/fspy_preload_windows/src/windows/client.rs
@graphite-app graphite-app Bot changed the base branch from 10-16-fix_task_use_shm_for_fspy_ipc to graphite-base/239 October 22, 2025 12:36
@branchseer branchseer force-pushed the 10-21-refactor_fspy_use_shared_memory_ipc_on_windows branch from 789dfb1 to 3be5ec4 Compare October 23, 2025 01:59
@branchseer branchseer force-pushed the 10-21-refactor_fspy_use_shared_memory_ipc_on_windows branch from 3be5ec4 to e58577d Compare October 23, 2025 02:01
@branchseer branchseer changed the base branch from graphite-base/239 to 10-16-fix_task_use_shm_for_fspy_ipc October 23, 2025 02:01
@branchseer branchseer changed the base branch from 10-16-fix_task_use_shm_for_fspy_ipc to graphite-base/239 October 23, 2025 11:42
branchseer and others added 5 commits October 23, 2025 11:50
Create named shared memory mappings that can be opened from child processes
without backing file access. This fixes rust_std tests on Windows where DLLs
injected via Detours need to access the IPC channel.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@branchseer branchseer force-pushed the 10-21-refactor_fspy_use_shared_memory_ipc_on_windows branch from e58577d to 72b1987 Compare October 23, 2025 11:50
@graphite-app graphite-app Bot changed the base branch from graphite-base/239 to main October 23, 2025 11:51
@branchseer branchseer force-pushed the 10-21-refactor_fspy_use_shared_memory_ipc_on_windows branch from 72b1987 to 7d676a0 Compare October 23, 2025 11:51
@branchseer branchseer merged commit 861e518 into main Oct 23, 2025
10 checks passed
Copy link
Copy Markdown
Member Author

Merge activity

@branchseer branchseer deleted the 10-21-refactor_fspy_use_shared_memory_ipc_on_windows branch October 23, 2025 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants