Skip to content

Fix lost thread and fiber locals#497

Merged
jkeen merged 3 commits into
graphiti-api:mainfrom
MattFenelon:Fix-lost-thread-and-fiber-locals
May 20, 2025
Merged

Fix lost thread and fiber locals#497
jkeen merged 3 commits into
graphiti-api:mainfrom
MattFenelon:Fix-lost-thread-and-fiber-locals

Conversation

@MattFenelon

Copy link
Copy Markdown
Contributor

Thread and fiber locals are being cleared on completion of every future. If the promise ran on the same thread as the request, or the same thread as the last promise, the locals are then not available. The execution context hasn't changed in those circumstances so the code that sets the locals at the start of a promise doesn't run.

  1. Thread 1 runs
  2. Sets context
  3. Loads scope
  4. Clears context
  5. Thread 1 runs next scope
  6. Context hasn’t changed so doesn’t set it again
  7. Loads scope
  8. Fails because the code relying on the context causes an exception because the locals aren't available.

Instead always set the thread and fiber locals and only clear those that we know are new. This should avoid the lost locals and also avoid memory leaks between promises.

See: #496

Thread and fiber locals are being cleared on completion of every future.
If the promise ran on the same thread as the request, or the same thread
as the last promise, the locals are then not available. The execution
context hasn't changed in those circumstances so the code that sets the
locals at the start of a promise doesn't run.

1. Thread 1 runs
2. Sets context
3. Loads scope
4. Clears context
5. Thread 1 runs next scope
6. Context hasn’t changed so doesn’t set it again
7. Loads scope
8. Fails because the code relying on the context causes an exception
   because the locals aren't available.

Instead always set the thread and fiber locals and only clear those that
we know are new. This should avoid the lost locals and also avoid
memory leaks between promises.

See: graphiti-api#496
@jkeen jkeen merged commit 5f45f76 into graphiti-api:main May 20, 2025
47 checks passed
@jkeen

jkeen commented May 20, 2025

Copy link
Copy Markdown
Collaborator

Thanks @MattFenelon! This looks/works great

github-actions Bot pushed a commit that referenced this pull request May 20, 2025
## [1.8.2](v1.8.1...v1.8.2) (2025-05-20)

### Bug Fixes

* prevent context loss by always setting thread and fiber locals ([#497](#497)) ([5f45f76](5f45f76))
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 1.8.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@MattFenelon

Copy link
Copy Markdown
Contributor Author

Good to hear! Thanks for the merge, @jkeen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants