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.

Sign up for GitHub

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

Jump to bottom

Fix busy-loop when waiting for file descriptors to close #7865

Merged
merged 3 commits into from
May 26, 2023
Merged

Fix busy-loop when waiting for file descriptors to close #7865

merged 3 commits into from
May 26, 2023

Commits on May 26, 2023

  1. Move rb_thread_cond_struct definition into thread_native.h 

    On Win32, currently, rb_nativethread_cond_t is an incomplete type
because it's a typedef for `struct rb_thread_cond_struct`. That means
you can't actually allocate a rb_nativethread_cond_t unless you also
include THREAD_IMPL_H (since its defined in thread_win32.h)
(alternatively, including vm_core.h also works).

Move the definition of rb_thread_cond_struct into thread_native.h to
alleviate this.
    @KJTsanaktsidis
    KJTsanaktsidis committed May 26, 2023
    Configuration menu
    Copy the full SHA
    3bde398 View commit details
    Browse the repository at this point in the history

  2. Fix busy-loop when waiting for file descriptors to close 

    When one thread is closing a file descriptor whilst another thread is
concurrently reading it, we need to wait for the reading thread to be
done with it to prevent a potential EBADF (or, worse, file descriptor
reuse).

At the moment, that is done by keeping a list of threads still using the
file descriptor in io_close_fptr. It then continually calls
rb_thread_schedule() in fptr_finalize_flush until said list is empty.

That busy-looping seems to behave rather poorly on some OS's,
particulary FreeBSD. It can cause the TestIO#test_race_gets_and_close
test to fail (even with its very long 200 second timeout) because the
closing thread starves out the using thread.

To fix that, I introduce the concept of struct rb_io_close_wait_list; a
list of threads still using a file descriptor that we want to close. We
call `rb_notify_fd_close` to let the thread scheduler know we're closing
a FD, which fills the list with threads. Then, we call
rb_notify_fd_close_wait which will block the thread until all of the
still-using threads are done.

This is implemented with a condition variable sleep, so no busy-looping
is required.
    @KJTsanaktsidis
    KJTsanaktsidis committed May 26, 2023
    Configuration menu
    Copy the full SHA
    874657c View commit details
    Browse the repository at this point in the history

  3. Remvoe very high timeout on test_race_gets_and_close 

    This test should be fixed and fast now because the closing thread sleeps
appropriately waiting for the file descriptor to be unused.
    @KJTsanaktsidis
    KJTsanaktsidis committed May 26, 2023
    Configuration menu
    Copy the full SHA
    0fc9e46 View commit details
    Browse the repository at this point in the history