Commit ba479a7b authored by Kenton Varda's avatar Kenton Varda

Remove mention of threads from some comments.

parent 2dbf31da
......@@ -194,8 +194,7 @@ SegmentBuilder* BasicBuilderArena::getSegment(SegmentId id) {
BasicBuilderArena::AllocateResult BasicBuilderArena::allocate(WordCount amount) {
if (segment0.getArena() == nullptr) {
// We're allocating the first segment. We don't need to worry about threads at this point
// because calling MessageBuilder::initRoot() from multiple threads is not intended to be safe.
// We're allocating the first segment.
kj::ArrayPtr<word> ptr = message->allocateSegment(amount / WORDS);
// Re-allocate segment0 in-place. This is a bit of a hack, but we have not returned any
......@@ -204,7 +203,7 @@ BasicBuilderArena::AllocateResult BasicBuilderArena::allocate(WordCount amount)
kj::ctor(segment0, this, SegmentId(0), ptr, &this->dummyLimiter);
return AllocateResult { &segment0, segment0.allocate(amount) };
} else {
// Check if there is space in the first segment. We can do this without locking.
// Check if there is space in the first segment.
word* attempt = segment0.allocate(amount);
if (attempt != nullptr) {
return AllocateResult { &segment0, attempt };
......@@ -242,8 +241,7 @@ BasicBuilderArena::AllocateResult BasicBuilderArena::allocate(WordCount amount)
// getSegmentsForOutput(), which callers might reasonably expect is a thread-safe method.
segmentState->forOutput.resize(segmentState->builders.size() + 1);
// Allocating from the new segment is guaranteed to succeed since no other thread could have
// received a pointer to it yet (since we still hold the lock).
// Allocating from the new segment is guaranteed to succeed since we made it big enough.
return AllocateResult { result, result->allocate(amount) };
}
}
......
......@@ -459,11 +459,9 @@ public:
kj::Own<CallContextHook>&& context) override {
auto contextPtr = context.get();
// We don't want to actually dispatch the call synchronously, because:
// 1) The server may prefer a different EventLoop.
// 2) If the server is in the same EventLoop, calling it synchronously could be dangerous due
// to risk of deadlocks if it happens to take a mutex that the client already holds. One
// of the main goals of message-passing architectures is to avoid this!
// We don't want to actually dispatch the call synchronously, because we don't want the callee
// to have any side effects before the promise is returned to the caller. This helps avoid
// race conditions.
//
// So, we do an evalLater() here.
//
......
......@@ -742,11 +742,7 @@ private:
~ImportClient() noexcept(false) {
unwindDetector.catchExceptionsIfUnwinding([&]() {
// Remove self from the import table, if the table is still pointing at us. (It's possible
// that another thread attempted to obtain this import just as the destructor started, in
// which case that other thread will have constructed a new ImportClient and placed it in
// the import table. Therefore, we must actually verify that the import table points at
// this object.)
// Remove self from the import table, if the table is still pointing at us.
KJ_IF_MAYBE(import, connectionState->imports.find(importId)) {
KJ_IF_MAYBE(i, import->importClient) {
if (i == this) {
......@@ -1002,8 +998,7 @@ private:
};
kj::Maybe<ExportId> writeDescriptor(ClientHook& cap, rpc::CapDescriptor::Builder descriptor) {
// Write a descriptor for the given capability. The tables must be locked by the caller and
// passed in as a parameter.
// Write a descriptor for the given capability.
// Find the innermost wrapped capability.
ClientHook* inner = &cap;
......@@ -1174,10 +1169,7 @@ private:
// is known that no more caps will be extracted after this point, so an exact value can be
// returned. Otherwise, the returned size includes room for error.
// If `final` is true then there's no need to lock. If it is false, then asynchronous
// access is possible. It's probably not worth taking the lock to look; we'll just return
// a silly estimate.
uint count = final ? retainedCaps.size() : 32;
uint count = retainedCaps.size() + (final ? 0 : 32);
return (count * sizeof(ExportId) + (sizeof(word) - 1)) / sizeof(word);
}
......@@ -1197,7 +1189,6 @@ private:
// Build the list of export IDs found in this message which are to be retained past the
// message's release.
// Called on finalization, when all extractions have ceased, so we can skip the lock.
kj::Vector<ExportId> retainedCaps = kj::mv(this->retainedCaps);
kj::Vector<kj::Own<ClientHook>> refs(retainedCaps.size());
......@@ -1391,8 +1382,8 @@ private:
}
void finishDescriptors() {
// Finish writing all of the CapDescriptors. Must be called with the tables locked, and the
// message must be sent before the tables are unlocked.
// Finish writing all of the CapDescriptors. The message containing the injected
// capabilities must be sent immediately after this returns.
exports = kj::Vector<ExportId>(caps.size());
......@@ -2313,14 +2304,12 @@ private:
QuestionId questionId = call.getQuestionId();
// Note: resolutionChainTail couldn't possibly be changing here because we only handle one
// message at a time, so we can hold off locking the tables for a bit longer.
auto context = kj::refcounted<RpcCallContext>(
*this, questionId, kj::mv(message), call.getParams(),
kj::addRef(*resolutionChainTail),
redirectResults, kj::mv(cancelPaf.fulfiller));
// No more using `call` after this point!
// No more using `call` after this point, as it now belongs to the context.
{
auto& answer = answers[questionId];
......@@ -2565,8 +2554,6 @@ private:
// Level 1
void handleResolve(const rpc::Resolve::Reader& resolve) {
kj::Own<ResolutionChain> oldResolutionChainTail; // must be freed outside of lock
kj::Own<ClientHook> replacement;
// Extract the replacement capability.
......@@ -2584,8 +2571,7 @@ private:
}
// Extend the resolution chain.
oldResolutionChainTail = kj::mv(resolutionChainTail);
resolutionChainTail = oldResolutionChainTail->addResolve(
resolutionChainTail = resolutionChainTail->addResolve(
resolve.getPromiseId(), kj::mv(replacement));
// If the import is on the table, fulfill it.
......
......@@ -159,8 +159,6 @@ public:
// If `firstSegmentWordSize` is non-zero, it should be treated as a hint suggesting how large
// to make the first segment. This is entirely a hint and the connection may adjust it up or
// down. If it is zero, the connection should choose the size itself.
//
// Notice that this may be called from any thread.
virtual kj::Promise<kj::Maybe<kj::Own<IncomingRpcMessage>>> receiveIncomingMessage() = 0;
// Wait for a message to be received and return it. If the read stream cleanly terminates,
......
......@@ -688,10 +688,10 @@ class WeakFulfiller final: public PromiseFulfiller<T>, private kj::Disposer {
// - We cannot destroy the WeakFulfiller until the application has discarded it *and* it has been
// detached from the underlying fulfiller, because otherwise the later detach() call will go
// to a dangling pointer. Essentially, WeakFulfiller is reference counted, although the
// refcount never goes over 2 and we manually implement the refcounting because we already need
// a mutex anyway. To this end, WeakFulfiller is its own Disposer -- dispose() is called when
// the application discards its owned pointer to the fulfiller and detach() is called when the
// promise is destroyed.
// refcount never goes over 2 and we manually implement the refcounting because we need to do
// other special things when each side detaches anyway. To this end, WeakFulfiller is its own
// Disposer -- dispose() is called when the application discards its owned pointer to the
// fulfiller and detach() is called when the promise is destroyed.
public:
KJ_DISALLOW_COPY(WeakFulfiller);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment