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

Safer effectAsync #1399

Closed
ghostdogpr opened this issue Aug 14, 2019 · 4 comments · Fixed by #1402
Closed

Safer effectAsync #1399

ghostdogpr opened this issue Aug 14, 2019 · 4 comments · Fixed by #1402

Comments

@ghostdogpr
Copy link
Member

Currently if the callback from effectAsync is called twice, it can lead to very unexpected results: a ClassCastException, or worse, calling the callback of another effectAsync (super hard to debug, nonsensical things may happen).

Sometimes, the fault is not even on user code but in libraries, for example http4s (https://github.com/http4s/http4s/blob/master/async-http-client/src/main/scala/org/http4s/client/asynchttpclient/AsyncHttpClient.scala (cb can be called twice in case of timeout in the underlying netty).

It would be nice if ZIO could protect from this kind of mistake, by allowing an effectAsync callback to be executed ONLY if it matches the current stack state. If not matching, it would be discarded (maybe logged) but wouldn't affect the current stack.

cc @jdegoes

Solving this would solve #857 as well.

@ghostdogpr
Copy link
Member Author

Actually there is a law in cats-effect called repeatedCallbackIgnored, so it might be the expected behavior from http4s.

@joroKr21
Copy link
Contributor

Somehow this law is passing in cats-interop though. We've had a similar issue with Scalaz task, it's handled by an atomic boolean in shims: https://github.com/djspiewak/shims/blob/master/effect/src/main/scala/shims/effect/instances/TaskInstances.scala#L44-L51

@ghostdogpr
Copy link
Member Author

@joroKr21 I think the law passes because the test is too simple. The effect of the second call depends on what’s in the stack. Interesting to see how shims handles it!

@regiskuckaertz
Copy link
Member

It sounds like what you need is a guarantee the callback is called exactly once, there's an interesting technique to enforce this kind of constraint with no runtime overhead. In particular, the VST monad tracks the number of times a resource is accessed. Perhaps useful 🤔

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 a pull request may close this issue.

3 participants