Skip to content

fix: guard TIOCGWINSZ before winpty_set_size to avoid assert on non-tty stdin - #212

Open
davidcoleman007 wants to merge 1 commit into
rprichard:masterfrom
davidcoleman007:fix/tiocgwinsz-guard
Open

fix: guard TIOCGWINSZ before winpty_set_size to avoid assert on non-tty stdin#212
davidcoleman007 wants to merge 1 commit into
rprichard:masterfrom
davidcoleman007:fix/tiocgwinsz-guard

Conversation

@davidcoleman007

@davidcoleman007 davidcoleman007 commented May 21, 2026

Copy link
Copy Markdown

Summary

When the unix-adapter is invoked with stdin that is not a tty (e.g. piped from a parent process such as an editor's terminal harness, a CI runner, or any non-interactive tool that wraps bash), ioctl(STDIN_FILENO, TIOCGWINSZ, ...) fails with ENOTTY. Both call sites in src/unix-adapter/main.cc ignored the return value, leaving the local winsize variable uninitialised. In the resize loop, the subsequent memcmp against the prior size fires spuriously and winpty_set_size(wp, sz.ws_col, sz.ws_row, NULL) ends up being called with cols == 0 && rows == 0, which trips the assertion in libwinpty:

Assertion failed: ASSERT_CONDITION("wp != nullptr && cols > 0 && rows > 0"),
file src/libwinpty/winpty.cc, line 924

This causes a stream of assertion banners on stderr from any wrapper that pipes bash through winpty without a real console.

Fix

Guard both ioctl(TIOCGWINSZ) call sites: only act on the result when the call succeeds and ws_col/ws_row are positive. The libwinpty invariant in winpty_set_size is correct as-is — the bug was in the caller passing zero dimensions.

  • Hunk A (initial-size, line 595) — defensive: re-fall back to the 80x25 default if the ioctl returned an error or zero-sized struct. The pre-init to 80x25 already covered most cases, so this is mostly symmetry with Hunk B.
  • Hunk B (resize loop, lines 691-694) — the actual crash path. Skip the winpty_set_size call entirely if the ioctl failed or reported a degenerate size.

Test plan

  • Built locally on a Git for Windows SDK / MSYS2 MinGW64 toolchain (gcc 15.2.0).
  • Replaced the system winpty binaries; reproduced previous behaviour with the unpatched version (assert spam) and confirmed it is silent with the patched version.
  • Verified normal interactive use (winpty cmd.exe /c echo hello, winpty --version) still works in mintty.

…ty stdin

When stdin is not a tty (e.g. piped from a parent process), ioctl(TIOCGWINSZ)
fails and leaves the winsize struct uninitialised. The subsequent memcmp
fires spuriously and winpty_set_size(wp, 0, 0) hits the assertion at
libwinpty/winpty.cc:924.

Fix: only call winpty_set_size when ioctl succeeds and cols/rows are > 0.
Apply the same guard to the initial-size path for symmetry.
@davidcoleman007

Copy link
Copy Markdown
Author

@rprichard thanks for the work on this package!

I don't know if this is actively developed these days, but i have been having the above issue consistently when using claude in git bash and windows git sdk. I patched it, tested with both sdk and git bash.

Please let me know if there is some additional process to certify this fix?

@rprichard

rprichard commented May 21, 2026 via email

Copy link
Copy Markdown
Owner

@davidcoleman007

davidcoleman007 commented May 21, 2026

Copy link
Copy Markdown
Author

Sorry, I haven't been maintaining this project since 2017 or so. Software that uses winpty ought to switch to using the official Windows ConPTY API instead, but ConPTY wasn't added until (some version of) Windows 10.

That's a real shame. Interestingly the current git bash distribution shipped by git/Microsoft today is actively including this package, and NOT ConPTY.

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.

2 participants