Skip to content

Commit

Permalink
A couple of minor improvements to AsyncContextFrame::StorageKey
Browse files Browse the repository at this point in the history
* Reset the tracing storage key in IoContext when the context is
  destroyed.
* Do not store a key if the key has already been reset
* Compare pointers and not hash in operator=
  • Loading branch information
jasnell committed Apr 20, 2023
1 parent cf63ae0 commit 1487d2a
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
4 changes: 4 additions & 0 deletions src/workerd/jsg/async-context.c++
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ AsyncContextFrame::AsyncContextFrame(Lock& js, StorageEntry storageEntry)
}
}

// This case is extremely unlikely to happen but let's handle it anyway
// just out of an excess of caution.
if (storageEntry.key->isDead()) return;

storage.upsert(kj::mv(storageEntry), [](StorageEntry& existing, StorageEntry&& row) mutable {
existing.value = kj::mv(row.value);
});
Expand Down
7 changes: 4 additions & 3 deletions src/workerd/jsg/async-context.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,11 @@ class AsyncContextFrame final: public Wrappable {
// An opaque key that identifies an async-local storage cell within the frame.
public:
StorageKey() : hash(kj::hashCode(this)) {}
KJ_DISALLOW_COPY_AND_MOVE(StorageKey);

void reset() { dead = true; }
// The owner of the key should reset it when it goes away.
// The StorageKey is typically owned by an instance of AsyncLocalstorage (see
// The StorageKey is typically owned by an instance of AsyncLocalStorage (see
// the api/node/async-hooks.h). When the ALS instance is garbage collected, it
// must call reset to signal that this StorageKey is "dead" and can never be
// looked up again. Subsequent accesses to a frame will remove dead keys from
Expand All @@ -83,7 +84,7 @@ class AsyncContextFrame final: public Wrappable {
bool isDead() const { return dead; }
inline uint hashCode() const { return hash; }
inline bool operator==(const StorageKey& other) const {
return hash == other.hash;
return this == &other;
}

private:
Expand Down Expand Up @@ -181,7 +182,7 @@ class AsyncContextFrame final: public Wrappable {
}

bool matches(const StorageEntry& entry, StorageKey& key) const {
return entry.key->hashCode() == key.hashCode();
return entry.key.get() == &key;
}

uint hashCode(StorageKey& key) const {
Expand Down

0 comments on commit 1487d2a

Please # to comment.