Skip to content

Fix resource leaks and unsafe shell commands in Python scripts#2722

Closed
shbhmexe wants to merge 2 commits intosu2code:developfrom
shbhmexe:fix/pythonresourceleaks
Closed

Fix resource leaks and unsafe shell commands in Python scripts#2722
shbhmexe wants to merge 2 commits intosu2code:developfrom
shbhmexe:fix/pythonresourceleaks

Conversation

@shbhmexe
Copy link

@shbhmexe shbhmexe commented Feb 1, 2026

This PR addresses critical resource leaks and unsafe coding practices in the Python wrapper scripts:

  • Resource Leaks: In SU2_PY/SU2/io/config.py, file operations in read_config, write_config, and dump_config were opening files without closing them. These have been refactored to use try...finally blocks or context managers (with open(...)) to ensure files are closed properly, preventing file descriptor exhaustion.
  • Unsafe Shell Commands: In SU2_PY/SU2/eval/functions.py and SU2_PY/SU2/eval/gradients.py, symbolic links were being created using subprocess.Popen(['ln', '-s', ...]). This is platform-dependent and unreliable. These have been replaced with the portable os.symlink function, including checks for existing links.

Related Work

  • Fixes potential file handle leaks identified during code analysis.
  • Improves cross-platform compatibility by removing direct shell command dependencies for symlinking.

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
  • My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
  • I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.

- Refactored `SU2/io/config.py` to use `try...finally` blocks for file handling, ensuring proper resource cleanup.
- Replaced unsafe `subprocess.Popen` calls for creating symbolic links in `SU2/eval/functions.py` and `SU2/eval/gradients.py` with `os.symlink`.
- Added checks for existing links before creation to prevent errors.

Signed-off-by: shbhmexe <shubhushukla586@gmail.com>
if not this_con:
continue # if no definition
# defaults
this_obj = "NONE"

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'this_obj' is unnecessary as it is
redefined
before this value is used.
this_obj = "NONE"
this_sgn = "="
this_scl = 1.0
this_val = 0.0

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'this_val' is unnecessary as it is
redefined
before this value is used.
# split across equals sign
line = line.split("=")
this_param = line[0].strip()
old_value = line[1].strip()

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable old_value is not used.
if "TARGET_CL" not in data_dict:
data_dict["TARGET_CL"] = 0.0
if "FREESTREAM_PRESSURE" not in data_dict:
data_dict["FREESTREAM_PRESSURE"] = 101325.0

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'multipoints' is unnecessary as it is
redefined
before this value is used.
This assignment to 'multipoints' is unnecessary as it is
redefined
before this value is used.
@shbhmexe
Copy link
Author

shbhmexe commented Feb 2, 2026

?

@pcarruscag
Copy link
Member

Buddy I told you before we are not machines.
I reviewed and approved the pull request, why the passive-aggressive ?? Is 24h not fast enough for you?
Well no problem, please find another project to contribute to, I will henceforth close any and all pull requests you open.

@pcarruscag pcarruscag closed this Feb 2, 2026
@shbhmexe
Copy link
Author

shbhmexe commented Feb 2, 2026

Buddy I told you before we are not machines. I reviewed and approved the pull request, why the passive-aggressive ?? Is 24h not fast enough for you? Well no problem, please find another project to contribute to, I will henceforth close any and all pull requests you open.

Apologies, sir. No passive aggression intended. Thank you for reviewing and approving the PR.

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.

2 participants