Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reactor: eager close of resource fds in the pending destruction list #98

Merged
merged 1 commit into from
Nov 19, 2024

Conversation

ymmt2005
Copy link
Member

@ymmt2005 ymmt2005 commented Nov 8, 2024

The file descriptor of a resource in the pending destruction list
may not be closed. This commit closes them when the list is fixed
for the next GC, adding more chance to close file descritors early.

The file descriptor of a resource in the pending destruction list
may not be closed. This commit closes them when the list is fixed
for the next GC, adding more chance to close file descritors early.
@ymmt2005 ymmt2005 self-assigned this Nov 8, 2024
@ymmt2005 ymmt2005 requested a review from nojima November 8, 2024 12:34
@nojima
Copy link
Collaborator

nojima commented Nov 19, 2024

@ymmt2005
I would like to clarify the purpose of this change.

If I am correct, fix_garbage is not called when object GC runs longer than gc_interval because handler->on_master_interval starts object GC if it can, and then reactor_gc_ready returns false. Therefore, I don't think this is a solution to #97.

@ymmt2005
Copy link
Member Author

ymmt2005 commented Nov 19, 2024

@nojima
Correct. This is unrelated to #97.
This merely adds more chances to close file descriptors.

@nojima
Copy link
Collaborator

nojima commented Nov 19, 2024

@ymmt2005 I got it

@nojima
Copy link
Collaborator

nojima commented Nov 19, 2024

Looks good to me

@ymmt2005
Copy link
Member Author

Thank you for your review!

@ymmt2005 ymmt2005 merged commit 90bdefe into master Nov 19, 2024
6 checks passed
@ymmt2005 ymmt2005 deleted the eager-close branch November 19, 2024 11:42
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.

2 participants