Skip to content

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.

… 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>
@shbhmexe shbhmexe changed the base branch from master to develop December 22, 2025 09:11
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 shbhmexe changed the title fix: avoid SU2_PY runner hangs on stderr output + guard config filename copies fix: prevent SU2_PY runner stderr deadlock and improve error handling Dec 24, 2025
@shbhmexe
Copy link
Author

Update the title and description please

Hi @pcarruscag ,

Updated the title and description. Thanks for the review!

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