Skip to content

Deduplicate PATHEXT extensions when resolving fallbacks#1937

Merged
oferchen merged 1 commit into
masterfrom
implement-and-optimize-oc-rsync-features
Nov 4, 2025
Merged

Deduplicate PATHEXT extensions when resolving fallbacks#1937
oferchen merged 1 commit into
masterfrom
implement-and-optimize-oc-rsync-features

Conversation

@oferchen
Copy link
Copy Markdown
Owner

@oferchen oferchen commented Nov 4, 2025

Summary

  • ensure Windows PATHEXT parsing removes duplicate extensions in different casings
  • reuse the same deduplication when seeding default extensions so fallback search stays stable
  • add a Windows-only regression test covering PATHEXT case-folding behaviour

Testing

  • cargo test -p rsync-core fallback_binary_candidates_deduplicates_duplicate_path_entries

https://chatgpt.com/codex/tasks/task_e_6909b6f6f6d08323bd33c4e94f188fea

@oferchen oferchen merged commit f34a25a into master Nov 4, 2025
@oferchen oferchen deleted the implement-and-optimize-oc-rsync-features branch November 4, 2025 08:49
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 on lines +224 to +228
let mut normalized = Vec::with_capacity(trimmed.len());
normalized.extend(trimmed.iter().copied().map(ascii_uppercase_u16));

if seen.insert(normalized) {
exts.push(OsString::from_wide(trimmed));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Deduping PATHEXT removes required case variants on case-sensitive paths

The new HashSet in push_segment uppercases each PATHEXT segment and drops any later entry whose case differs only by ASCII (seen.insert(normalized)). On Windows directories that have case sensitivity enabled, the file system treats rsync.exe and rsync.EXE as distinct files. Users sometimes include both .exe and .EXE in PATHEXT to probe both cases. After this change only the first casing is ever tried, so a fallback binary that exists solely as the second casing will be skipped even though it is present on disk. The previous implementation probed all case variants and succeeded in this scenario, so this is a regression that can prevent fallback lookups from finding an installed binary on case-sensitive directories.

Useful? React with 👍 / 👎.

oferchen added a commit that referenced this pull request May 5, 2026
…3716)

Static-analysis audit recording the runtime model decision for the
io_uring integration. Recommends keeping io_uring synchronous and
routing async callers through tokio::task::spawn_blocking (#1751)
rather than adopting tokio-uring or building a custom poll-driven
SQE submitter, on the grounds that fast_io must remain tokio-free
and the session ring pool design (#1937) stays simpler when leases
are synchronous.
oferchen added a commit that referenced this pull request May 7, 2026
Plan replaces std and tokio TcpStream halves in the daemon accept
loop with the io_uring socket reader/writer adapters once the shared
session ring (#1874) and per-session ring pool (#1936/#1937) land,
reusing the POLLOUT linked-timeout fix from #1872.
oferchen added a commit that referenced this pull request May 7, 2026
Concrete plan to finish #1408/#1409: lookup `RingPool` by `SessionId`,
tie ring lifetime to session drop via `SessionRegistry::unregister`,
and migrate sockets, disk_batch, and copy_file_range to the shared
path. Cites #1936 for the per-session lookup contract.
oferchen added a commit that referenced this pull request May 7, 2026
…3894)

Document that buffer-group ids are u16 in io_uring (max 65 535 per
ring), audit current `BufferRing` lifecycle (kernel reclaims via
`IORING_UNREGISTER_PBUF_RING` on drop, but no in-process allocator
owns the namespace), sketch the long-running daemon worst case, and
propose a hard-capped slab allocator with telemetry and a typed
exhaustion error before #1936 / #1937 wire PBUF_RING through the
per-session ring.
oferchen added a commit that referenced this pull request May 17, 2026
Rewrite the session-ring-pool design doc around the daemon-session
framing: pool keyed by SessionId, lazy init, max-size and idle eviction,
hazards around kernel-object lifetime and registered-fd reuse, and a
bench plan that isolates session-startup latency. Recommend deferring
implementation until #2243 (per-thread rings) lands so the pool layer
does not inherit the SharedRing Arc<Mutex<RawIoUring>> bottleneck noted
in #1876.
oferchen added a commit that referenced this pull request May 17, 2026
…4232)

Records the signal layer, kernel constraints, and concrete trigger
thresholds for adaptive sizing of the registered buffer pool under
sustained pressure. Covers gaps left by the existing companion
documents: CQE wait time, slot exhaustion counter, sustained
backpressure window, IORING_REGISTER_BUFFERS_UPDATE shrink fast
path, and cgroup v2 memory headroom. Recommends deferring the
implementation until the session ring pool (#1937) lands so the
sizer is tuned against the multi-ring workload that motivates it.
oferchen added a commit that referenced this pull request May 17, 2026
Introduces SessionRingPool, a small fleet of long-lived io_uring rings
that consumers lease for one submit/reap cycle, amortising
io_uring_setup(2) and IORING_REGISTER_* costs across daemon sessions.

The primitive lives in crates/fast_io/src/io_uring/session_pool.rs on
Linux with a stub in io_uring_stub.rs for other targets. Selection is
round-robin via a single relaxed AtomicUsize; per-ring contention falls
on individual std::sync::Mutex slots. Drop releases every ring
deterministically, including any SQPOLL kthread.

Existing shared_ring.rs consumers stay on the single-mutex pattern.
Migrations land one consumer at a time in follow-ups per the design
plan in docs/design/iouring-session-ring-pool-impl.md.

API surface:
- SessionRingPool::{new,try_new,acquire,acquire_slot,ring_count,config}
- SessionPoolConfig with from_io_uring_config / with_ring_count helpers
- RingLease: RAII handle dereferencing to &mut IoUring
oferchen added a commit that referenced this pull request May 17, 2026
… (#4285)

Audits whether prerequisites for #1876 have landed and proposes
splitting the work into reviewable subtasks (#1876-a through e).

- #1874 (shared ring + POLL_ADD) shipped via PR #3553.
- #1937 (session ring pool) shipped via PR #4275.
- #1935 (hybrid tokio listener) shipped via PR #4278.
- No io_uring task carries id #1875; the original dependency note
  most plausibly meant the session-pool and listener work above.

Missing surface enumerated: pooled-ring constructors on the socket
adapters, daemon plumbing for SessionRingPool, sync and async worker
swaps, IORING_OP_LINK_TIMEOUT integration, and async-cancel
discipline. Trigger conditions (kernel >= 5.6, io_uring feature,
opt-in flag) and a five-step implementation plan included.
oferchen added a commit that referenced this pull request May 17, 2026
Catalogues the allocation site (BgidAllocator::allocate at
crates/fast_io/src/io_uring/buffer_ring.rs:193) and the release site
(BufferRing::Drop at buffer_ring.rs:720, deallocate at :245).
Pins BgidExhausted definition (io_uring_common.rs:370) and the two
raise sites. Identifies the gap that BGE-3 and BGE-4 need to close:
no peak telemetry, unbounded free-list, no in-tree caller (production
PBUF wiring is still pending under #1936/#1937). Lays out the
free-list pool + Drop signal recycling plan for BGE-4 and the
churn / loom / recovery stress matrix for BGE-5.
oferchen added a commit that referenced this pull request May 18, 2026
…3716)

Static-analysis audit recording the runtime model decision for the
io_uring integration. Recommends keeping io_uring synchronous and
routing async callers through tokio::task::spawn_blocking (#1751)
rather than adopting tokio-uring or building a custom poll-driven
SQE submitter, on the grounds that fast_io must remain tokio-free
and the session ring pool design (#1937) stays simpler when leases
are synchronous.
oferchen added a commit that referenced this pull request May 18, 2026
Plan replaces std and tokio TcpStream halves in the daemon accept
loop with the io_uring socket reader/writer adapters once the shared
session ring (#1874) and per-session ring pool (#1936/#1937) land,
reusing the POLLOUT linked-timeout fix from #1872.
oferchen added a commit that referenced this pull request May 18, 2026
Concrete plan to finish #1408/#1409: lookup `RingPool` by `SessionId`,
tie ring lifetime to session drop via `SessionRegistry::unregister`,
and migrate sockets, disk_batch, and copy_file_range to the shared
path. Cites #1936 for the per-session lookup contract.
oferchen added a commit that referenced this pull request May 18, 2026
…3894)

Document that buffer-group ids are u16 in io_uring (max 65 535 per
ring), audit current `BufferRing` lifecycle (kernel reclaims via
`IORING_UNREGISTER_PBUF_RING` on drop, but no in-process allocator
owns the namespace), sketch the long-running daemon worst case, and
propose a hard-capped slab allocator with telemetry and a typed
exhaustion error before #1936 / #1937 wire PBUF_RING through the
per-session ring.
oferchen added a commit that referenced this pull request May 18, 2026
Rewrite the session-ring-pool design doc around the daemon-session
framing: pool keyed by SessionId, lazy init, max-size and idle eviction,
hazards around kernel-object lifetime and registered-fd reuse, and a
bench plan that isolates session-startup latency. Recommend deferring
implementation until #2243 (per-thread rings) lands so the pool layer
does not inherit the SharedRing Arc<Mutex<RawIoUring>> bottleneck noted
in #1876.
oferchen added a commit that referenced this pull request May 18, 2026
…4232)

Records the signal layer, kernel constraints, and concrete trigger
thresholds for adaptive sizing of the registered buffer pool under
sustained pressure. Covers gaps left by the existing companion
documents: CQE wait time, slot exhaustion counter, sustained
backpressure window, IORING_REGISTER_BUFFERS_UPDATE shrink fast
path, and cgroup v2 memory headroom. Recommends deferring the
implementation until the session ring pool (#1937) lands so the
sizer is tuned against the multi-ring workload that motivates it.
oferchen added a commit that referenced this pull request May 18, 2026
Introduces SessionRingPool, a small fleet of long-lived io_uring rings
that consumers lease for one submit/reap cycle, amortising
io_uring_setup(2) and IORING_REGISTER_* costs across daemon sessions.

The primitive lives in crates/fast_io/src/io_uring/session_pool.rs on
Linux with a stub in io_uring_stub.rs for other targets. Selection is
round-robin via a single relaxed AtomicUsize; per-ring contention falls
on individual std::sync::Mutex slots. Drop releases every ring
deterministically, including any SQPOLL kthread.

Existing shared_ring.rs consumers stay on the single-mutex pattern.
Migrations land one consumer at a time in follow-ups per the design
plan in docs/design/iouring-session-ring-pool-impl.md.

API surface:
- SessionRingPool::{new,try_new,acquire,acquire_slot,ring_count,config}
- SessionPoolConfig with from_io_uring_config / with_ring_count helpers
- RingLease: RAII handle dereferencing to &mut IoUring
oferchen added a commit that referenced this pull request May 18, 2026
… (#4285)

Audits whether prerequisites for #1876 have landed and proposes
splitting the work into reviewable subtasks (#1876-a through e).

- #1874 (shared ring + POLL_ADD) shipped via PR #3553.
- #1937 (session ring pool) shipped via PR #4275.
- #1935 (hybrid tokio listener) shipped via PR #4278.
- No io_uring task carries id #1875; the original dependency note
  most plausibly meant the session-pool and listener work above.

Missing surface enumerated: pooled-ring constructors on the socket
adapters, daemon plumbing for SessionRingPool, sync and async worker
swaps, IORING_OP_LINK_TIMEOUT integration, and async-cancel
discipline. Trigger conditions (kernel >= 5.6, io_uring feature,
opt-in flag) and a five-step implementation plan included.
oferchen added a commit that referenced this pull request May 18, 2026
Catalogues the allocation site (BgidAllocator::allocate at
crates/fast_io/src/io_uring/buffer_ring.rs:193) and the release site
(BufferRing::Drop at buffer_ring.rs:720, deallocate at :245).
Pins BgidExhausted definition (io_uring_common.rs:370) and the two
raise sites. Identifies the gap that BGE-3 and BGE-4 need to close:
no peak telemetry, unbounded free-list, no in-tree caller (production
PBUF wiring is still pending under #1936/#1937). Lays out the
free-list pool + Drop signal recycling plan for BGE-4 and the
churn / loom / recovery stress matrix for BGE-5.
oferchen added a commit that referenced this pull request May 18, 2026
Rewrite the session-ring-pool design doc around the daemon-session
framing: pool keyed by SessionId, lazy init, max-size and idle eviction,
hazards around kernel-object lifetime and registered-fd reuse, and a
bench plan that isolates session-startup latency. Recommend deferring
implementation until #2243 (per-thread rings) lands so the pool layer
does not inherit the SharedRing Arc<Mutex<RawIoUring>> bottleneck noted
in #1876.
oferchen added a commit that referenced this pull request May 18, 2026
…4232)

Records the signal layer, kernel constraints, and concrete trigger
thresholds for adaptive sizing of the registered buffer pool under
sustained pressure. Covers gaps left by the existing companion
documents: CQE wait time, slot exhaustion counter, sustained
backpressure window, IORING_REGISTER_BUFFERS_UPDATE shrink fast
path, and cgroup v2 memory headroom. Recommends deferring the
implementation until the session ring pool (#1937) lands so the
sizer is tuned against the multi-ring workload that motivates it.
oferchen added a commit that referenced this pull request May 18, 2026
Introduces SessionRingPool, a small fleet of long-lived io_uring rings
that consumers lease for one submit/reap cycle, amortising
io_uring_setup(2) and IORING_REGISTER_* costs across daemon sessions.

The primitive lives in crates/fast_io/src/io_uring/session_pool.rs on
Linux with a stub in io_uring_stub.rs for other targets. Selection is
round-robin via a single relaxed AtomicUsize; per-ring contention falls
on individual std::sync::Mutex slots. Drop releases every ring
deterministically, including any SQPOLL kthread.

Existing shared_ring.rs consumers stay on the single-mutex pattern.
Migrations land one consumer at a time in follow-ups per the design
plan in docs/design/iouring-session-ring-pool-impl.md.

API surface:
- SessionRingPool::{new,try_new,acquire,acquire_slot,ring_count,config}
- SessionPoolConfig with from_io_uring_config / with_ring_count helpers
- RingLease: RAII handle dereferencing to &mut IoUring
oferchen added a commit that referenced this pull request May 18, 2026
… (#4285)

Audits whether prerequisites for #1876 have landed and proposes
splitting the work into reviewable subtasks (#1876-a through e).

- #1874 (shared ring + POLL_ADD) shipped via PR #3553.
- #1937 (session ring pool) shipped via PR #4275.
- #1935 (hybrid tokio listener) shipped via PR #4278.
- No io_uring task carries id #1875; the original dependency note
  most plausibly meant the session-pool and listener work above.

Missing surface enumerated: pooled-ring constructors on the socket
adapters, daemon plumbing for SessionRingPool, sync and async worker
swaps, IORING_OP_LINK_TIMEOUT integration, and async-cancel
discipline. Trigger conditions (kernel >= 5.6, io_uring feature,
opt-in flag) and a five-step implementation plan included.
oferchen added a commit that referenced this pull request May 18, 2026
Catalogues the allocation site (BgidAllocator::allocate at
crates/fast_io/src/io_uring/buffer_ring.rs:193) and the release site
(BufferRing::Drop at buffer_ring.rs:720, deallocate at :245).
Pins BgidExhausted definition (io_uring_common.rs:370) and the two
raise sites. Identifies the gap that BGE-3 and BGE-4 need to close:
no peak telemetry, unbounded free-list, no in-tree caller (production
PBUF wiring is still pending under #1936/#1937). Lays out the
free-list pool + Drop signal recycling plan for BGE-4 and the
churn / loom / recovery stress matrix for BGE-5.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant