tests #2
Open
tests #2
Conversation
This file contains 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
A buffer was read after being freed when two unusual things happened together: 1) Some of the initial connection content arrived immediately with the HTTP response headers, causing there to be a `leftover` buffer. This is rare because the response headers would normally be returned when the connection was opened, but the nature of TCP doesn't allow the server to return any bytes until an additional network round trip has occurred. 2) The application tried to read with a `minBytes` greater than the size of this leftover. (This is unusual because most use cases set `minBytes` to 1, which can't possibly be larger than a non-empty leftover size.) In this case, the leftover's backing buffer was destroyed, but the ArrayPtr pointing into it was not reset, so the next read would try to read it again. I have not observed this problem happening in practice. I just noticed the bug while reading the code.
String, StringPtr, Array, ArrayPtr gain as<T>() method which is a syntax sugar for T::from(this). This enables defining conversion domains (such as Std in tests) and use `.as<Std>()` chained calls for more fluid expressions.
In this mode, InMemoryDirectory works as normal, but when it creates a File, instead of using InMemoryFile, it uses DiskFile wrapping a memfd. This creates a nice compromise for writing tests of code that depends on files being backed by real file descriptors, but does not need the same for directories. (If we wanted similar functionality on other operating systems, we could create an InMemoryFileFactory that backs files with anonymous temporary files on disk...)
json-rpc and json-rpc-test were not included in bazel so far. Adding the test to bazel resolves a long-standing internal issue.
I've long considered this library a mistake. It hides too much of the internals, which leads people to get stuck quickly as soon as they actually need those internals. It's much better to use `kj::setupAsyncIo()` to set up the KJ event loop / networking, and then `capnp::TwoPartyServer` or `capnp::TwoPartyClient` to set up RPC.
Plain old `thread_local` should be fine these days.
The implementation of `transfer()` would deadlock if the source and destination directories were the same object, since it would attempt to operate on `fromDirectory` while holding the lock on the destination directory. Fixing this required a significant rewrite of this function.
Allow InMemoryDirectory to be backed by memfds
I'm not sure why I ever designed it this way, but LowLevelAsyncIoProviderImpl has always owned the UnixEventPort, EventLoop, and WaitScope. This means you actually couldn't use the main async I/O implementation while also controlling allocation of these objects, which is silly.
This makes it possible to use UnixEventPort inside a thread running some other event loop, by arranging for that other event loop to listen for the UnixEventPort's epoll FD becoming ready. Whenever it does, the application must pump the KJ event loop. This also could allow multiple event loops to run in the same thread, or even across some thread pool, as long as only one event loop is active in a thread a time, and each event loop is active in no more than one thread at a time. A scheduler could schedule event loops to threads when their epoll FD becomes ready.
Also, threads changing EventLoops.
I found some of these while reading https://github.com/capnproto/capnproto/pull/1889/files and ispell found the rest.
I noticed this while reading https://github.com/capnproto/capnproto/pull/1889/files.
…gardening Typos and whitespace fixes for the filesystem files
Delete ez-rpc and KJ_THREADLOCAL_PTR
Extend UnixEventPort on Linux to allow external polling.
Fix leftover handling in HTTP CONNECT implementation.
…ges. Renamed some things from "pong" to "control message"; the things that are actually pong-specific remain.
Addresses a long standing todo in workerd where we duplicate GlobFilter there.
Allows a type to be explicitly marked in a way that prevent
it from being passed into a kj promise coroutine as an arg.
```
class Foo {
private:
KJ_DISALLOW_AS_COROUTINE_PARAM;
};
kj::Promise<void> simpleCoroutine(Foo foo) { // Compile Error!
co_return;
}
```
…n frame.
Previously, WebSocketImpl::receive() would just throw an exception
when this happened, which resulted in the destruction of the WebSocket
and the closing of its underlying TCP connection. The WebSocket client
didn't get any indication of what they did wrong.
Now, the client gets a Close frame with error code 1002 (terminating
due to protocol error) and a reason ("Unexpected continuation frame"),
so they can learn what they did wrong.. WebSocketImpl::receive still
throws, but it waits to do so until after the Close has been sent.
There are a bunch of different protocol errors that deserve Close
frames, but this commit only addresses one so that the changes to
WebSocketImpl are easier to review.
Protocol errors are things like a client setting RSV bits 2 or 3, sending a compressed message when no compression was negotiated, and so on.
This is a step towards getting rid of RequestState.
We can't quite remove the TaskSet itself yet, since disconnectWebSocket() still uses it.
Instead, we change CapnpToKjWebSocketAdapter to pass off responsibility for the `disconnect()` call to its creator. This seems like a better design -- we can now actually explicitly wait for the stream to complete. Previously I think it was possible that we'd kill the websocket prematurely if it had a bunch of messages queued up still upon stream completion.
Instead, since this is the only remaining user of any of the cancellation functionality, we simply merge it into this class itself. I think the code ends up somewhat simpler, though I think it could get even moreso if it turns out `disconnect()` isn't really needed. I think this may be the case: WebSocket connections already have an explicit close message and are not intended to run with the socket itself in a half-open state. So maybe we could just remove `disconnect()` entirely and close the underlying socket in the destructor instead. But that's a change that extends outside http-over-capnp, so not doing it here.
If attempting to send the reply back to the client fails, we should cancel out the whole request and propagate the exception, rather than uselessly info-log it.
…over-capnp Cleanup and simplify http-over-capnp code.
This reverts commit 20be408.
Revert "Revert "Make getPreferredExtensions pure virtual func""
nhynes
force-pushed
the
nhynes/vsock
branch
2 times, most recently
from
9eab58a
to
ef32386
Compare
This constructor is currently unused, so the current issue has no immediate impact. Signed-off-by: Jianyong Chen <baluschch@gmail.com>
kj/table: fix the initialization of BTreeImpl::MaybeUnit with uint.
Fixes potential read-after-free that can happen if the connection's shutdown() method throws an exception.
…ception capnp-rpc: retain RpcConnectionState reference in error handling lambda
check for empty AggregateConnectionReceiver
nitpick: fix a few typos in the comments
This error log is necessarily followed by a use-after-free later on that can be nasty. We should crash eagerly instead.
…imit-uaf In ~ConcurrencyLimitingHttpClient(), crash more eagerly.
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.
No description provided.