a bunch of fixes for nodejs #204
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
UD doesn't come with an error code.
We shouldn't consider the reuse flags when determining whether a port is a candidate for an ephemeral port, but we still need to store them in the entry.
For UDP sockets, sending a packet can trigger an implicit bind operation. This requires knowing the user id. This can happen through the write and send syscall families.
It turns out that waiting for confirmation every time is *really* slow. I saw 70ms roundtrips for a simple WAIT+WAKE combo. The reasons we confirmed wake ops in the first place was cancellations: If a WAIT operation timed out and got woken at the same time, there was a race condition where the timeout error was returned and the wake operation was dropped. This lead to deadlocks. Instead, we now atomically cancel the futex and check if it was woken up before returning a timeout error. Note that there's still a chance for wakups to get dropped when the process is killed, but that's not as much of an issue. If it becomes an issue, we could consider detecting this situation and waking up another WAIT op.
rlimit access require 16-byte atomic accesses and x86 natively supports these, so let's not use a lock.
Next up, we'll remove some of the fields and reimplement the accessors.
The vast majority of accesses only need to read not write.
A decent chunk of file access contexts never look at the credentials, so let's hold off on fetching them until actually required.
We'll need to implement this eventually, but for now, let's just not panic.
This limit is not yet enforced.
Notify::wait is often called in a loop. This change makes it possible to reuse the same NotifyWait instance for multiple iterations. This is a bit faster because we can skip the setup code whenever an instance is reused. It's also a bit more convenient and it's now possible to call until directly on the returned Future. This change optimizes Notify further by moving the generation counter outside the Mutex and replacing it with an AtomicU64. The generation counter is still only written whenever the lock is taken, but this approach allows for optimistic reads without taking a lock. In a lot of cases, polling the NotifyWait doesn't require *any* writes ( including writes necessary for taking locks) which should help greatly reduce contention. Writes are only necessary when updating the list of wakers.
Node JS depends on file descriptors owned by a thread being closed before a signal about the processes's death is sent to the parent.
The upcoming epoll changes need direct access to the Notify instances.
Closing a socket works slightly differently if not all bytes have been read yet.
The old implementation had a few problems: 1. It didn't support edge-triggered events. Some applications require edge-triggered events, level triggered events won't do. 2. It didn't support cyclying through entries if there are more entries than requested. The new implementation supports both of these. By default, only level- triggered events are supported for file descriptors. Edge-triggered events require an epoll_ready implementation that also returns an edge- counter for each event type.
The new implementation only polls entries that have notified their waker. It doesn't poll every future every time.
Previously we always searched the ephemeral ports in order. Some applications open and close a bunch of sockets without being mindful of the scenario that the socket might have already been bound recently. Using a less sequential allocation scheme helps avoid issues without port-reuse.
Some applications don't work if the hostname is "host". That seems a bit odd, but then again, it's not like this is important, so let's just change it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR has a bunch of different fixes that make all Node.js tests pass.