Fix extremely slow ttyname_r performance on macOS#315
Closed
Fix extremely slow ttyname_r performance on macOS#315
Conversation
ab55df5 to
3e08a6b
Compare
On macOS, the ttyname_r system call can take 2-5+ seconds instead of milliseconds, causing severe performance issues when retrieving passwords (issue doy#11). This commit uses fcntl with F_GETPATH on macOS to get the TTY device path, which is much faster. The fcntl approach returns the same TTY path (e.g., /dev/ttys001) that ttyname_r would return, maintaining correct behavior for multi-terminal scenarios where the agent is started in one terminal and unlock is requested from another. Changes: - Check if stdin is a terminal before attempting to get its path - On macOS: Use fcntl(F_GETPATH) to get TTY path (fast) - Other platforms: Continue using rustix::termios::ttyname (standard) - Falls back to ttyname if fcntl fails on macOS Testing shows password retrieval time reduced from 2-5+ seconds to ~0.2 seconds on macOS. Fixes doy#11 Note: This solution was implemented with assistance from Claude Sonnet 4.5.
9b31ff1 to
25c9d8a
Compare
…vulnerabilities (RUSTSEC-2026-0007, RUSTSEC-2026-0049)
|
Couldn't you implement this upstream in termios rather than just rbw? |
Author
|
@sewnie was right — this fix belongs upstream in rustix rather than here.
I've opened a PR upstream: bytecodealliance/rustix#1605 I tested a local build of rbw against the patched rustix and confirmed it's fast. Closing this in favor of the upstream fix. |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note: This solution was implemented with assistance from Claude Sonnet 4.5. I don't know rust (yet). Feel free to close out.
Problem
On macOS, the
ttyname_rsystem call can take 2-5+ seconds instead of milliseconds, causing severe performance degradation when retrieving passwords. This affects users with large password databases where the delay becomes particularly noticeable.Fixes #11
Solution
This PR uses
fcntlwithF_GETPATHon macOS to retrieve the TTY device path, which is much faster thanttyname_r. The implementation:fcntl(F_GETPATH)to get the TTY path (fast)rustix::termios::ttyname(standard approach)Why This Addresses the Maintainer's Concern
The maintainer previously raised a concern about using
/dev/tty(which would refer to the agent's terminal rather than the client's terminal). This solution does not use/dev/tty. Instead:fcntl(F_GETPATH)is called on stdin of therbwclient process (the terminal where you runrbw get)/dev/ttys001or/dev/ttys002This is exactly what
ttyname_rwas doing - we're just using a faster macOS-specific API to achieve the same result.Multi-Terminal Scenario
This correctly handles the case where:
/dev/ttys002) is detected and passed to the agentTesting
Technical Details
The
fcntlcall withF_GETPATH(value 50 on macOS) is a macOS-specific API that retrieves the filesystem path for a file descriptor. It's documented in the macOS fcntl man page and is the recommended approach for this use case on macOS.Note: This solution was implemented with assistance from Claude Sonnet 4.5.