Skip to content

Conversation

@pull
Copy link

@pull pull bot commented Apr 28, 2021

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

bnoordhuis and others added 27 commits July 29, 2024 16:30
Under rare but benign circumstances (on XNU), incoming datagrams appear
to be dropped by the operating system after libuv has been notified of
their arrival but before libuv has had a chance to receive them.

Fixes: #4219
The test is skipped in its entirety on OpenBSD so there is no point in
compiling out code on said platform later on, it's not run anyway.
Implements posix delete for files and dirs, with fallback to the old
method if not supported (e.g. Fat32 or Win8).

Fixes: #3839
If the corresponding environment variables are empty, the
uv_us_homedir() and uv_os_tmpdir() return garbage values. The reason
for this situation is the Windows API which doesn't return an error
even if the path is empty.

This PR fixes this problem by checking the return value of the API
call. If it is not an error and the length of the value is less than 3,
uv_us_homedir() and uv_os_tmpdir() will return UV_ENOENT.

Fixes: #2328
Which was broken both in `windows` and `macos`.
Register the eventfd with EPOLLET to enable edge-triggered notification
where we're able to eliminate the overhead of reading the eventfd via
system call on each wakeup event.

When the eventfd counter reaches the maximum value of the unsigned 64-bit,
which may not happen for the entire lifetime of the process, we rewind the
counter and retry.

This optimization saves one system call on each event-loop wakeup,
eliminating the overhead of read(2) as well as the extra latency
for each epoll wakeup.
Windows added a new API for file information, which doesn't have to
open the file thus greatly improving performance:
https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-getfileinformationbyname

The stat functions are already covered by tests, so no test was added
here. I considered comparing the result of old and new code, but that
would require exposing internal fs functions, and we would be testing
Windows functionality, not libuv.
This fixes a race condition if multiple threads are reading from the
same NamedPipe, which could previously lead to a deadlock situation. We
also substantially improve performance now also, since the PeekFile
call is unnecessary overhead with this change. This API was added in
Windows Vista.

Related to #4467, though doesn't address any of the problems there. I
believe that someone could now implement uv__pipe_try_write using
this same code pattern however.
The Event should be reset before reading the value, or libuv might miss
an update that occurred too rapidly after the previously one.

Refs: #2381
Refs: #4485
MSVC does not actually support ubsan. There is a long-standing ticket
requesting this:
https://developercommunity.visualstudio.com/t/add-support-for-ubsan/840750

There are no known compilers that currently accept the
`/fsanitize=undefined` spelling. clang-cl accepts `-fsanitize...`,
same as regular clang.

Also passes no-sanitizer-recover so that tests actually fail.

Fix various ubsan-detected errors, including:

* win: fix req-inl.h ubsan failure

Don't use CONTAINING_RECORD macro from WinSDK, as it doesn't use the
right trick which avoids member access on null pointer.

Fixes:
```
src/win/req-inl.h:86:10: runtime error: member access within null pointer of type 'uv_req_t' (aka 'struct uv_req_s')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior D:/a/libuv/libuv/src/win/req-inl.h:86:10
```

* test: fix ubsan failure on udp_ref3

Don't call functions through different function type.

Fixes:
```
src/win/udp.c:537:5: runtime error: call to function req_cb through pointer to incorrect function type 'void (*)(struct uv_udp_send_s *, int)'
test\test-ref.c:66: note: req_cb defined here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/win/udp.c:537:5 in
```

* win: fix process-stdio.c ubsan failure

When accessing HANDLEs within the stdio buffer, use memcpy / memset in order to respect alignment.

Fixes:
```
src/win/process-stdio.c:197:5: runtime error: store to misaligned address 0x0230ee72d107 for type 'HANDLE' (aka 'void *'), which requires 8 byte alignment
0x0230ee72d107: note: pointer points here
  00 00 cd cd cd  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd fd  fd fd fd
              ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/win/process-stdio.c:197:5 in
```

* win: fix getaddrinfo.c ubsan failure

Reworks buffer alignment handling to respect requirements.

Fixes:
```
src/win/getaddrinfo.c:157:23: runtime error: member access within misaligned address 0x0290e4c6a17c for type 'struct addrinfo', which requires 8 byte alignment
0x0290e4c6a17c: note: pointer points here
  00 00 00 00 cd cd cd cd  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd  cd cd cd cd cd cd cd cd
              ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/win/getaddrinfo.c:157:23 in
```

* win: fix pipe.c ubsan failure

Changes "random" representation from pointer to number.

Fixes:
```
src/win/pipe.c:234:11: runtime error: applying non-zero offset to non-null pointer 0xffffffffffffffff produced null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/win/pipe.c:234:11 in
```

* unix: fix stream.c ubsan failure

Avoids performing pointer arithmetic on null pointer.

Fixes:
```
src/unix/stream.c:701:15: runtime error: applying zero offset to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/runner/work/libuv/libuv/src/unix/stream.c:701:15 in
```
The SQPOLL io_uring instance wasn't providing consistent behaviour to
users depending on kernel versions, load shape, ... creating issues
difficult to track and fix. Don't use this ring by default but allow
enabling it by calling `uv_loop_configure()` with
`UV_LOOP_ENABLE_IO_URING_SQPOLL`.
Refactor / cleanup arithmetic for unix -> win filetime conversion
in order to avoid multiplication overflow.

Fixes:
```
src/win/fs.c:106:48: runtime error: signed integer overflow: 1702781567 * 10 cannot be represented in type 'long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/win/fs.c:106:48 in
```

Co-authored-by: Jameson Nash <[email protected]>
POSIX allows `rmdir` to return `EEXIST` or `ENOTEMPTY` for a non-empty
directory, so the test needs to allow both.
All other checks for `UV_RENAME` in `test-fs-event` also allow
`UV_CHANGE`.
Autobinding is a feature that lets the kernel pick a name for the
abstract socket, instead of userspace having to provide one.

Two bugs that this change exposed are also fixed:

1. strlen(sa.sun_path) can read past the end if the file path is exactly
   sizeof(sa.sun_path) long (use memchr instead), and

2. don't return UV_ENOBUFS for abstract sockets when the buffer is
   exactly large enough to hold the result; per commit e5f4b79,
   abstract socket names are not zero-terminated
Establishes a user event for kqueue to eliminate the overhead
of the pipe and the system call read(2) per wakeup event.

---------

Signed-off-by: Andy Pan <[email protected]>

Co-authored-by: Jameson Nash <[email protected]>
In #4470, I accidentally copied the bug from unix, where calling
uv_stream_set_blocking can cause the whole process to hang on a read.
However, unlike unix, where libuv attempts to set the O_NONBLOCK flag in
uv_pipe_open (as long as the handle never gets passed to uv_spawn), the
NT kernel is not capable of enabling OVERLAPPED operation later (but
fortunately, it also cannot disable it later too).

This implementation might be good to copy to unix (using FIONREAD) to
address the same bug that happens there if the user has called uv_spawn
or uv_stream_set_non_blocking on this handle in the past.
vtjnash and others added 30 commits September 25, 2025 11:02
While unlikely that this fails (since it is implemented to call
`abort()` instead of actually return an error code, we would
theoretically leak this state. The process state init doesn't actually
have a cleanup, and just awkwardly relies on uv_signal_close being
unnecessary (which is likely true though).
#4901)

The test previously assumed a default nice value of 0, but this can vary
depending on PAM configuration, /etc/security/limits.conf, or other environment
settings.

Fixes: #4898

Signed-off-by: Juan José Arboleda <[email protected]>
Fixes: #4551

Signed-off-by: Juan José Arboleda <[email protected]>
Add a stub function for no-ahafs builds. src/unix/core.c won't actually
call uv__ahafs_event but the linker still needs to see a symbol.
Fixes: #4447
Signed-off-by: Juan José Arboleda <[email protected]>
Co-authored-by: theanarkh <[email protected]>
Add a stub function for no-ahafs builds. src/unix/core.c won't actually
call uv__ahafs_event but the linker still needs to see a symbol.

Like commit 87943b0 but for IBM i instead of AIX this time.
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 5 to 6.
- [Release notes](https://github.com/actions/download-artifact/releases)
- [Commits](actions/download-artifact@v5...v6)

---
updated-dependencies:
- dependency-name: actions/download-artifact
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4 to 5.
- [Release notes](https://github.com/actions/upload-artifact/releases)
- [Commits](actions/upload-artifact@v4...v5)

---
updated-dependencies:
- dependency-name: actions/upload-artifact
  dependency-version: '5'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
The `flags` argument to `uv__udp_recvmsg_errqueue` was not initialized
in all code paths. Shuffle code around to make the control flow more
obvious (at the cost of a less legible diff.)

Also fixes an unused label warning when building on systems that aren't
Linux.

Introduced last month in commit 80a5e3b, "linux: add MSG_ERRQUEUE
ipv4/ipv6 udp support".
When `wtf8_ptr` is NULL, the length will be stored in `wtf8_len_ptr` (not in `wtf8_ptr`).

Fixes: #4934
Bumps [actions/checkout](https://github.com/actions/checkout) from 5 to 6.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v5...v6)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Fetch file metadata by handle instead of by path, otherwise there is a
race window between fetching and acting on said metadata where another
process can replace the file with another one.

There is still a potential race when upgrading the short path to a long
path but that that one is intrinsic. It's not something libuv can solve
except by refraining from calling GetLongPathName.

Fixes: #4568
Remove the microsecond truncation in uv__fs_to_timespec() function to enable
full nanosecond resolution support. The utimensat() system call already
supports nanosecond precision, so the artificial truncation to microseconds
is no longer necessary.

This allows file modification and access times to be set with full nanosecond
precision instead of being limited to microsecond resolution.

Fixes the TODO comment about removing the microsecond resolution limit.
The initial FindFirstFile can fail with ERROR_FILE_NOT_FOUND, meaning
no matches (read: empty directory), which leaves dir_handle set to
INVALID_HANDLE_VALUE. Not an actual error, it just means no results.

I can't get FindFirstFile to work like that on regular file systems
but it's been reported that it does under sshfs-win and the MSDN
documentation clearly states it's possible. Handle it.

Fixes: #4952
Added `TEST_FS_DECLARE`, `TEST_FS_ENTRY` and `TEST_FS_IMPL` so we can
run test-fs.c tests both using the threadpool and io_uring
implementations.
The `offset` field should be assigned to ithe `io_uring_sqe.off` field.
Add macos-15 runners. Remove the deprecated macos-13 runners.

Fixes: #4965
Refs: actions/runner-images#10924
And fix a stupid typo that made GHA skip running the tests 🤦
Use GetFileInformationByHandleEx(FileBasicInfo) instead of GetFileInformationByHandle() because it's cheaper -- one syscall, instead of two [1]

[1]: https://blog.axiorema.com/engineering/hidden-cost-getfileinformationbyhandle/
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 6 to 7.
- [Release notes](https://github.com/actions/download-artifact/releases)
- [Commits](actions/download-artifact@v6...v7)

---
updated-dependencies:
- dependency-name: actions/download-artifact
  dependency-version: '7'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 5 to 6.
- [Release notes](https://github.com/actions/upload-artifact/releases)
- [Commits](actions/upload-artifact@v5...v6)

---
updated-dependencies:
- dependency-name: actions/upload-artifact
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Fix `threadpool_cancel_fs_iouring` so it takes `UV_FS_FTRUNCATE` into
account.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.