No caller reads the pthread_t, so raddrinfo_thread_create() no longer takes one, and a non-pthread implementation can sit behind it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An invalid open_timeout raised before the waiter was assigned, so the cleanup closed fd 0 left by memset and took the process's stdin with it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hostname is added as a suffix to getaddrinfo error messages. for
example, a "name or service not known" error message becomes "name or
service not known for `invalidthost.com`.
The lengths passed to recv(2), send(2) and gethostname(2), and the lengths of sockaddr, are left as they are.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
EAI_ADDRFAMILY left the IPv6 result looking successful, so a later IPv4 resolution error was not recorded and TCPSocket.new raised Socket::ResolutionError with error_code 0. When the IPv6 notification failed, EAI_ADDRFAMILY was recorded instead and hid the IPv4 error.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The loop only left on EAGAIN, so EOF or a read error other than EINTR spun forever while holding the GVL.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
When stdin is closed, a socket can get fd 0, and nfds stayed 0 if it was the only fd to wait for, so select never reported the connection.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The fd was closed but left in connection_attempt_fds, so it was passed to the next select and closed again in the cleanup, possibly after the number had been reused.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
remove_connection_attempt_fd shifts the remaining fds down, so advancing the index skipped the fd moved into the current slot. A connection that had already succeeded could go unnoticed until the next select, and a new attempt was started meanwhile.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The main thread polls has_syserr on every loop while the resolver thread may be setting it, so read it with the lock the resolver thread holds. Once it is seen set, err and ai were written before it and can be read as before.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The resolver thread checks notify and writes to it while holding the lock. Closing it outside the lock let the thread write a byte to an unrelated file that had reused the fd number.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Only test_mode_settings itself was checked to be a Hash, and its delay and error values were passed to rb_hash_aref as they were.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The delay in `test_mode_settings` was stored as a raw VALUE, so a delay of n milliseconds slept for 2n+1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`fast fallback` cannot be used with explicitly specified local port,
because concurrent binds to the same `local_host:local_port`
can raise `Errno::EADDRINUSE`.
This issue is more likely to occur on hosts with `IPV6_V6ONLY` disabled,
because IPv6 binds can also occupy IPv4-mapped IPv6 address space.
This change fixes a bug in which specifying 0 for timeout-related options (such as the `timeout` option of `Addrinfo.getaddrinfo`) incorrectly results in an infinite wait.
(This change overwrites https://github.com/ruby/ruby/pull/15626 .)
This change adds host information to the error messages shown when a timeout occurs while passing timeout options to `TCPSocket.new` or `Socket.tcp`, for improved usability.
(When the `fast_fallback option` is enabled, there may be multiple possible destinations, so the host name is shown instead of an IP address.)
As part of this change, the error messages in `Addrinfo.getaddrinfo` and `Addrinfo#connect_internal`, both of which are used by `Socket.tcp`, have also been improved in the same way.
* `Socket.tcp` and `TCPSocket.new` raises `IO::TiemoutError` with user specified timeout
In https://github.com/ruby/ruby/pull/11880, `rsock_connect()` was changed to raise `IO::TimeoutError` when a user-specified timeout occurs.
However, when `TCPSocket.new` attempts to connect to multiple destinations, it does not use `rsock_connect()`, and instead raises `Errno::ETIMEDOUT` on timeout.
As a result, the exception class raised on timeout could differ depending on whether there were multiple destinations or not.
To align this behavior with the implementation of `rsock_connect()`, this change makes `TCPSocket.new` raise `IO::TimeoutError` when a user-specified timeout occurs.
Similarly, `Socket.tcp` is updated to raise `IO::TimeoutError` when a timeout occurs within the method.
(Note that the existing behavior of `Addrinfo#connect_internal`, which Socket.tcp depends on internally and which raises `Errno::ETIMEDOUT` on timeout, is not changed.)
* [ruby/net-http] Raise `Net::OpenTimeout` when `TCPSocket.open` raises `IO::TimeoutError`.
With the changes in https://github.com/ruby/ruby/pull/15602, `TCPSocket.open` now raises `IO::TimeoutError` when a user-specified timeout occurs.
This change updates #connect to handle this case accordingly.
https://github.com/ruby/net-http/commit/f64109e1cf
This change updates the behavior so that, when there is only a single destination and `open_timeout` is specified, the remaining `open_timeout` duration is used as the connection timeout.
We were seeing segfaults when calling `pthread_detach`. Apparently in
some versions of glibc there is a race between when this is called
(usually right after starting a thread) and a short-lived thread's
shutdown routine. The bug has been reported to glibc:
https://sourceware.org/bugzilla/show_bug.cgi?id=19951
I haven't been able to reproduce it on my Linux desktop but apparently
it's easier to reproduce on certain kinds of servers.
As a workaround, we can set the thread's detach state before thread
start. I don't know of a platform that doesn't have
`pthread_attr_setdetachstate`, but to be safe we check for it in
`extconf.rb` and use `pthread_detach` as a backup if it isn't available.
Fixes [Bug #21679]
This suppresses this warning:
../../../ext/socket/ipsocket.c: In function ‘rsock_raise_user_specified_timeout’:
../../../ext/socket/ipsocket.c:30:1: warning: function might be candidate for attribute ‘noreturn’ [-Wsuggest-attribute=noreturn]
30 | rsock_raise_user_specified_timeout()
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
* [Feature #21347] Add `open_timeout` as an overall timeout option for `TCPSocket.new`
With this change, `TCPSocket.new` now accepts the `open_timeout` option.
This option raises an exception if the specified number of seconds has elapsed since the start of the method call,
even if the operation is still in the middle of name resolution or connection attempts.
The addition of this option follows the same intent as the previously merged change to `Socket.tcp`.
[Feature #21347](https://bugs.ruby-lang.org/issues/21347)
https://github.com/ruby/ruby/pull/13368
* Tidy up: Extract rsock_raise_user_specified_timeout()
* Added a note to the documentation of `Socket.tcp`
* Fix `rsock_init_inetsock` for `FAST_FALLBACK_INIT_INETSOCK_IMPL`
[Bug #21506] Fix timeout in Addrinfo.getaddrinfo to actually take effect
This change fixes an issue where the timeout option in `Addrinfo.getaddrinfo` was not functioning as expected.
It also addresses a related issue where specifying `fast_fallback: false` with `resolv_timeout` for `Socket.tcp` and`TCPSocket.new` would have no effect.
The timeout option was originally introduced in:
https://github.com/ruby/ruby/commit/6382f5cc91ac9e36776bc854632d9a1237250da7
However, the value was noy used in current implementation:
https://github.com/ruby/ruby/blob/3f0e0d5c8bf9046aee7f262a3f9a7524d51aaf3e/ext/socket/raddrinfo.c#L1282-1308
Therefore, even if a timeout is specified and the duration elapses during name resolution, nothing happens. This is clearly not the intended behavior.
`Addrinfo.getaddrinfo` has been made interruptible as of Feature #19965.
This change uses that feature to interrupt name resolution when the specified timeout period elapses, raising a user-specified timeout error.
The timeout can be specified in milliseconds.
The same issue affects `Socket.tcp` and `TCPSocket.new` when `resolv_timeout` is set along with `fast_fallback: false`.
`resolv_timeout` was introduced in the following commits:
https://github.com/ruby/ruby/commit/6382f5cc91ac9e36776bc854632d9a1237250da7https://github.com/ruby/ruby/commit/511fe23fa2bdf1f17faa91e0558be47b5bb62b2a
The reason is that with `fast_fallback: false`, these methods internally call the same `rsock_getaddrinfo()` as `Addrinfo.getaddrinfo`.
This change addresses that as well.
This change addresses the following ASAN error:
```
==36597==ERROR: AddressSanitizer: heap-use-after-free on address 0x512000396ba8 at pc 0x7fcad5cbad9f bp 0x7fff19739af0 sp 0x7fff19739ae8
WRITE of size 8 at 0x512000396ba8 thread T0
[643/756] 36600=optparse/test_summary
#0 0x7fcad5cbad9e in free_fast_fallback_getaddrinfo_entry /home/runner/work/ruby-dev-builder/ruby-dev-builder/ext/socket/raddrinfo.c:3046:22
#1 0x7fcad5c9fb48 in fast_fallback_inetsock_cleanup /home/runner/work/ruby-dev-builder/ruby-dev-builder/ext/socket/ipsocket.c:1179:17
#2 0x7fcadf3b611a in rb_ensure /home/runner/work/ruby-dev-builder/ruby-dev-builder/eval.c:1081:5
#3 0x7fcad5c9b44b in rsock_init_inetsock /home/runner/work/ruby-dev-builder/ruby-dev-builder/ext/socket/ipsocket.c:1289:20
#4 0x7fcad5ca22b8 in tcp_init /home/runner/work/ruby-dev-builder/ruby-dev-builder/ext/socket/tcpsocket.c:76:12
#5 0x7fcadf83ba70 in vm_call0_cfunc_with_frame /home/runner/work/ruby-dev-builder/ruby-dev-builder/./vm_eval.c:164:15
...
```
A `struct fast_fallback_getaddrinfo_shared` is shared between the main thread and two child threads.
This struct contains an array of `fast_fallback_getaddrinfo_entry`.
`fast_fallback_getaddrinfo_entry` and `fast_fallback_getaddrinfo_shared` were freed separately, and if `fast_fallback_getaddrinfo_shared` was freed first and then an attempt was made to free a `fast_fallback_getaddrinfo_entry`, a `heap-use-after-free` could occur.
This change avoids that possibility by separating the deallocation of the addrinfo memory held by `fast_fallback_getaddrinfo_entry` from the access and lifecycle of the `fast_fallback_getaddrinfo_entry` itself.
* Do not save ResolutionError if resolution succeeds for any address family
Socket with Happy Eyeballs Version 2 performs connection attempts and name resolution in parallel.
In the existing implementation, if a connection attempt failed for one address family while name resolution was still in progress for the other, and that name resolution later failed, the method would terminate with a name resolution error.
This behavior was intended to ensure that the final error reflected the most recent failure, potentially overriding an earlier error.
However, [Bug #21088](https://bugs.ruby-lang.org/issues/21088) made me realize that terminating with a name resolution error is unnatural when name resolution succeeded for at least one address family.
This PR modifies the behavior so that if name resolution succeeds for one address family, any name resolution error from the other is not saved.
This PR includes the following changes:
* Do not display select(2) as the system call that caused the raised error, as it is for internal processing
* Fix bug: Get errno with Socket::SO_ERROR in Windows environment with a workaround for tests not passing
Rarely, there are cases where a write(2) call from a child thread
to notify the main thread of the completion of name resolution fails.
If this happens while the main thread is waiting in `rb_thread_fd_select`,
rb_thread_fd_select may not notice that the name resolution has completed and end up hanging.
This issue becomes a problem when there are no sockets currently being connected,
no addresses ready for immediate connection attempts,
and name resolution has already completed for one address family
while the main thread is waiting for the name resolution of the other address family.
(If name resolution is not completed for either address family,
the chances of write(2) failing in both child threads are likely low.)
To avoid this issue, a timeout is introduced to rb_thread_fd_select under the above conditions.
This way, even if the issue occurs,
the completion of name resolution should still be detected
in the subsequent `if (!resolution_store.is_all_finished) ...` block.
Wrap `do_fast_fallback_getaddrinfo` with `rb_thread_prevent_fork`
Referencing PR #10864,
wrap `do_fast_fallback_getaddrinfo` with `rb_thread_prevent_fork`
to avoid fork safety issues.
`do_fast_fallback_getaddrinfo` internally uses getaddrinfo(3),
leading to fork safety issues, as described in PR #10864.
This change ensures that `do_fast_fallback_getaddrinfo`
is guarded by `rb_thread_prevent_fork`,
preventing fork during its execution and avoiding related issues.
Any memory allocated with xmalloc needs to be matched with xfree rather
than plain free.
Ruby unfortunately redefines strdup to be ruby_strdup, which uses
xmalloc so needs to be xfreed. Previously these were mismatched.
This commit changes the copy to be an explicit ruby_strdup (to avoid
confusion) and the free to be xfree.
* Use `rb_thread_fd_select` instead of select(2)
For fixing https://bugs.ruby-lang.org/issues/20932 .
`TCPSocket.new`, which internally uses select(2) for HEv2, can cause SEGV if the number of file descriptors exceeds `FD_SETSIZE`.
This change avoids that issue by replacing select(2) with `rb_thread_fd_select`, which is provided as part of Ruby's internal API.
---
This includes the following changes.
* rb_thread_fd_select does not need common pipe
* Improve the conditions for clearing the Connection Attempt Delay upon connection failure
This change addresses a case that was overlooked in ruby/ruby#12087.
In the previous change, the Connection Attempt Delay was cleared at the point of a connection failure only if both of the following conditions were met:
- No other sockets were attempting a connection
- There were addresses still available to start a new connection
In this update, the second condition has been removed.
As a result, if name resolution succeeds after a connection failure and new addresses are obtained, it will be able to immediately attempt a connection to one of them.
If there are no sockets attempting a connection, no addresses available for connection, and name resolution has completed, an exception will still be raised as before.
---
Additionally, the following minor fixes have been made:
* Refactor: Remove unnecessary members
`TCPSocket.new` with HEv2 uses three threads.
The last of these threads to exit closed pipes.
However, if pipes were open at the end of the main thread, they would leak.
This change avoids this by closing pipes at the end of the main thread.
With https://github.com/ruby/ruby/pull/12156,
the memory of the `struct fast_fallback_getaddrinfo_shared`
is now allocated even if there is only one address family.
This change will always free it when `TCPSocket.new` finishes.
```
for (int i = 0; i < arg->family_size; i++) {
arg->getaddrinfo_entries[i] = allocate_fast_fallback_getaddrinfo_entry();
if (!(arg->getaddrinfo_entries[i])) rb_syserr_fail(errno, "calloc(3)");
```
If the allocation fails in the second interation, the memory allocated
in the first iteration would be leaked.
This change prevents the memory leak by allocating the memory in
advance.
(The struct name `fast_fallback_getaddrinfo_shared` might no longer be
good.)
* Do not save the last_error if there are no sockets waiting to be connected
In this implementation, the results of both name resolution and connection attempts are awaited using select(2).
When it returned, the implementation attempted to check for connections even if there were no sockets currently attempting to connect, treating the absence of connected sockets as a connection failure.
With this fix, it will no longer check for connections when there are no sockets waiting to be connected.
Additionally, the following minor fixes have been made:
* Handle failure of getsockopt(2) and removed unnecessary continue in the loop
* Tweak: Use common API to check in_progress_fds
* Safely call TCPServer.new in test
* Set empty writefds when there is no socket waiting to be connected
* Enable fast_fallback option
`rb_thread_call_without_gvl2` is used to wait for the results of name resolution and connection attempts.
When there is only one address family to resolve, the necessary resources were not being passed to the UBF.
With this change, the handling of resources has been revised and organized to work consistently, whether there are two address families to resolve or only one.
even if a system call error happens after the name resolution failure in the child thread.
pipe and write(2) are used to notify the main thread of the name resolution results from the child thread.
After name resolution is completed in the child thread, if the call to write(2) fails, the main thread retrieves the resolved addresses.
However, when name resolution failed, the corresponding error was not being saved in `last_error`.
With this change, name resolution failures will now be saved in last_error even if the write(2) call in the child thread fails.
http://ci.rvm.jp/results/trunk-repeat50@ruby-sp2-noble-docker/5420911
```
/tmp/ruby/src/trunk-repeat50/ext/socket/ipsocket.c: In function ‘reallocate_connection_attempt_fds’:
/tmp/ruby/src/trunk-repeat50/ext/socket/ipsocket.c:292:62: warning: pointer ‘fds’ may be used after ‘realloc’ [-Wuse-after-free]
292 | for (int i = current_capacity; i < new_capacity; i++) fds[i] = -1;
| ^
/tmp/ruby/src/trunk-repeat50/ext/socket/ipsocket.c:288:9: note: call to ‘realloc’ here
288 | if (realloc(fds, new_capacity * sizeof(int)) == NULL) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
http://ci.rvm.jp/results/trunk_asan@ruby-sp1/5409001
```
=================================================================
==3263562==ERROR: AddressSanitizer: stack-use-after-return on address 0x735a8f190da8 at pc 0x735a6f58dabc bp 0x735a639ffd10 sp 0x735a639ffd08
READ of size 4 at 0x735a8f190da8 thread T211
=================================================================
```
