Skip to content

fix: prevent SU2_PY runner stderr deadlock and improve error handling#2653

Merged
pcarruscag merged 6 commits intosu2code:developfrom
shbhmexe:bugfix/su2py-run-command-stderr-deadlock
Dec 24, 2025
Merged

fix: prevent SU2_PY runner stderr deadlock and improve error handling#2653
pcarruscag merged 6 commits intosu2code:developfrom
shbhmexe:bugfix/su2py-run-command-stderr-deadlock

Conversation

@shbhmexe
Copy link

@shbhmexe shbhmexe commented Dec 22, 2025

Proposed Changes

This PR fixes reliability issues in the SU2 Python runner (SU2_PY/SU2/run/interface.py):

  1. Stderr deadlock prevention - Use subprocess.communicate() to drain stderr while streaming stdout, avoiding potential hangs when SU2 processes write heavily to stderr.

  2. Improved environment variable handling - Added explicit error message when SU2_RUN environment variable is not set.

  3. Path reporting fix - Fixed typo in error messages (abspath(".") instead of abspath(",")) to show correct working directory.

Reasoning

  • The Python runner previously waited for process exit before reading stderr. If stderr fills its pipe buffer, the child process can block indefinitely.
  • Missing SU2_RUN now gives a clear error message instead of a confusing KeyError.

Impact

  • No behavior change for normal runs
  • Prevents rare but severe runner hangs on large stderr output
  • Better error messages for debugging

Verification

  • Tested: python -m compileall -q SU2_PY
  • No changes to solver algorithms or config parsing

PR Checklist

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings.
  • My contribution is commented and consistent with SU2 style.
  • I used the pre-commit hook to prevent dirty commits.
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation, if necessary.

shbhmexe added 2 commits December 22, 2025 14:38
… copies

- SU2_PY: use subprocess.communicate() to reliably drain stderr while keeping stdout streamed,
  avoiding potential hangs when stderr output is large.
- SU2_PY: fix incorrect path reporting in error messages (abspath(".") instead of abspath(",")).
- C++ entrypoints: guard/limit copies into fixed-size config filename buffers to prevent
  potential overflow on long paths.

Signed-off-by: shbhmexe <shubhushukla586@gmail.com>
Signed-off-by: shbhmexe <shubhushukla586@gmail.com>
Keeping only Python fixes:
- stderr deadlock prevention via subprocess.communicate()
- path reporting bug fix (abspath('.') not abspath(','))

Reverted strcpy  strncpy changes from SU2_CFD, SU2_DEF, SU2_DOT,
SU2_GEO, and SU2_SOL per reviewer's earlier feedback to keep it simple.

Signed-off-by: shbhmexe <shubhushukla586@gmail.com>
@shbhmexe
Copy link
Author

Hey @pcarruscag,
Would appreciate if you could take a look at this PR when you have time. #2650 , #2636
Thanks!

@pcarruscag
Copy link
Member

Update the title and description please

@shbhmexe
Copy link
Author

Update the title and description please

Hi @pcarruscag ,

Updated the title and description. Thanks for the review!

@pcarruscag pcarruscag merged commit bd4d182 into su2code:develop Dec 24, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants