Skip to content

Resolve capture_output(capture_fd=True) deadlock on Windows#3679

Merged
mrmundt merged 21 commits intoPyomo:mainfrom
jsiirola:tee-deadlock-win
Aug 8, 2025
Merged

Resolve capture_output(capture_fd=True) deadlock on Windows#3679
mrmundt merged 21 commits intoPyomo:mainfrom
jsiirola:tee-deadlock-win

Conversation

@jsiirola
Copy link
Copy Markdown
Member

@jsiirola jsiirola commented Aug 5, 2025

Fixes #3658

Summary/Motivation:

The recent rework of capture_output to make it more robust (#3583, #3588, #3601, #3633, #3640) led to a situation where using capture_output(capture_fd=True) on Windows would cause the process to deadlock (see #3658). We tracked it down to what appears to be Window's procedure for reallocating (growing) the buffer that underlies os.pipe(). This PR proposes a two-fold solution:

  1. Request a larger pipe buffer (64kb), to reduce the chances that the OS will ever need to reallocate the buffer
  2. Mark the pipe as being non-blocking (PIPE_NOWAIT), on the theory that losing log data is better than deadlocking the process

Changes proposed in this PR:

  • Provide windows-specific implementation of os.pipe() to request a larger buffer
  • Make the pipe underlying capture_output non-blocking

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

Copy link
Copy Markdown
Contributor

@mrmundt mrmundt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nasty.

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 6, 2025

Codecov Report

❌ Patch coverage is 33.33333% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.88%. Comparing base (adc610b) to head (32578aa).

Files with missing lines Patch % Lines
pyomo/common/tee.py 33.33% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3679      +/-   ##
==========================================
- Coverage   89.20%   87.88%   -1.32%     
==========================================
  Files         890      890              
  Lines      102777   102785       +8     
==========================================
- Hits        91678    90335    -1343     
- Misses      11099    12450    +1351     
Flag Coverage Δ
builders 26.75% <33.33%> (+<0.01%) ⬆️
default 85.77% <33.33%> (?)
expensive 34.11% <33.33%> (?)
linux 31.12% <33.33%> (-57.83%) ⬇️
linux_other 31.12% <33.33%> (-55.83%) ⬇️
osx ?
win ?
win_other ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@blnicho blnicho moved this from Todo to Reviewer Approved in August 2025 Release Aug 6, 2025
@jsiirola
Copy link
Copy Markdown
Member Author

jsiirola commented Aug 7, 2025

I am closing this for the time being. There are a number of issues that weren't showing up locally that need to be addressed before review.

@jsiirola jsiirola closed this Aug 7, 2025
@jsiirola jsiirola reopened this Aug 7, 2025
@jsiirola jsiirola requested a review from blnicho August 7, 2025 23:38
@jsiirola jsiirola requested a review from mrmundt August 8, 2025 03:48
@mrmundt mrmundt merged commit 35b2802 into Pyomo:main Aug 8, 2025
62 of 63 checks passed
@jsiirola jsiirola deleted the tee-deadlock-win branch August 8, 2025 22:18
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.

Regression in pyomo when using graybox models and cyipopt (Hanging solve)

3 participants