Skip to content

Conversation

@panjf2000
Copy link
Owner

No description provided.

Copilot AI review requested due to automatic review settings December 27, 2025 04:20
@panjf2000 panjf2000 changed the title bug: switch to socket.Dup in eventloop.enroll to enable close-on-exec (#745) bug: switch to socket.Dup in eventloop.enroll to enable close-on-exec Dec 27, 2025
@github-actions github-actions bot added the bug Something isn't working label Dec 27, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors file descriptor duplication to use socket.Dup instead of unix.Dup, automatically enabling the close-on-exec flag to prevent file descriptor leaks when forking processes.

  • Replaces unix.Dup with socket.Dup for automatic close-on-exec handling
  • Removes redundant manual unix.CloseOnExec calls in client code
  • Ensures consistent file descriptor duplication behavior across the codebase

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
eventloop_unix.go Updated enroll method to use socket.Dup for duplicating file descriptors with close-on-exec flag
client_unix.go Replaced unix.Dup + unix.CloseOnExec with socket.Dup call in EnrollContext method for cleaner implementation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Dec 27, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.30%. Comparing base (58d255f) to head (0c3e6f8).
⚠️ Report is 37 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #746      +/-   ##
==========================================
- Coverage   84.31%   84.30%   -0.01%     
==========================================
  Files          22       22              
  Lines        2403     2402       -1     
==========================================
- Hits         2026     2025       -1     
  Misses        257      257              
  Partials      120      120              
Flag Coverage Δ
unittests 84.30% <100.00%> (-0.01%) ⬇️

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.

@panjf2000 panjf2000 merged commit aa18b2f into master Dec 27, 2025
110 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants