Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create Warning Fiber In Test Clock Scope #7787

Merged
merged 6 commits into from
Feb 6, 2023
Merged

Create Warning Fiber In Test Clock Scope #7787

merged 6 commits into from
Feb 6, 2023

Conversation

adamgfraser
Copy link
Contributor

Resolves zio/zio-http#1896.

Currently in the TestClock we fork a warning fiber when a caller accesses the time that we interrupt when the caller actually adjusts the time or when the scope of the TestClock is closed.

However, this can create an issue if the warning fiber is forked in an inner scope where for example it uses an executor that is not valid outside that scope and the fiber is interrupted when the TestClock scope is closed after the inner scope is closed.

We can prevent this by forking the fiber at the time that the TestClock layer is constructed and merely completing a promise when we want to "start" the warning. This way we can ensure that the fiber is forked in a scope that is still valid when the fiber is finalized.

@frekw
Copy link
Contributor

frekw commented Feb 5, 2023

Wow, tricky! Great work @adamgfraser 🙏

@vigoo vigoo merged commit 9ba543f into zio:series/2.x Feb 6, 2023
@frekw
Copy link
Contributor

frekw commented Feb 6, 2023

Amazing, hopefully we can now we full ZIO 2.x support in Caliban (by updating zio-http). I'm really excited by that – do you know if there's currently an ETA for 2.0.7 (since 2.0.6 was ~3 weeks ago and there have been some bug fixes and performance improvements since 😃)?

@ghostdogpr
Copy link
Member

@frekw can you test our branch with the zio snapshot that will be created very soon? So we are 100% sure it's okay before a release 😄

@frekw
Copy link
Contributor

frekw commented Feb 6, 2023

@ghostdogpr haha I'm already sitting and impatiently following the CI run 😂

@frekw
Copy link
Contributor

frekw commented Feb 6, 2023

Aww CI seems to be failing so no snapshot was published :/

@adamgfraser adamgfraser deleted the testclock branch February 6, 2023 13:40
@frekw
Copy link
Contributor

frekw commented Feb 6, 2023

@ghostdogpr works great! 🎉 🎉 🎉

  + ZHttpAdapterSpec
    + http
      + test http endpoint
      + test interceptor failure
      + lower-case content-type header
    + test ws endpoint
      + legacy ws
      + graphql-ws
5 tests passed. 0 tests failed. 0 tests ignored.

Executed in 12 s 250 ms

[info] Completed tests
[success] Total time: 32 s, completed 6 Feb 2023, 15:40:38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Netty executor leaking outside zio-http
4 participants