Remove implicit forking in ZIO#onDone and ZIO#onDoneCause to align with onX methods#9286
Merged
kyri-petrou merged 14 commits intozio:series/2.xfrom Nov 21, 2024
Merged
Remove implicit forking in ZIO#onDone and ZIO#onDoneCause to align with onX methods#9286kyri-petrou merged 14 commits intozio:series/2.xfrom
kyri-petrou merged 14 commits intozio:series/2.xfrom
Conversation
Member
|
@asr2003 These need tests to guarantee synchronous behavior for each. |
Contributor
Author
|
@jdegoes Sure! I will add it |
kyri-petrou
reviewed
Nov 9, 2024
Contributor
kyri-petrou
left a comment
There was a problem hiding this comment.
@asr2003 in addition to John's comment, can you please do the following:
- Add scaladoc to both
onDoneandonDoneCausemethods - Write a paragraph explaining the behaviour change of
onDonewhich we will use in release notes. This should be similar to the "Important note about ZPool changes" found in the v2.1.10 notes
Contributor
|
One last comment, I think the implementation should use |
Contributor
Author
|
@kyri-petrou @jdegoes Done with the changes! |
kyri-petrou
reviewed
Nov 14, 2024
Co-authored-by: kyri-petrou <67301607+kyri-petrou@users.noreply.github.com>
Co-authored-by: kyri-petrou <67301607+kyri-petrou@users.noreply.github.com>
Contributor
Author
|
@kyri-petrou There has been type error if we use So I have updated for the same to resolve this type mismatch |
jdegoes
reviewed
Nov 15, 2024
kyri-petrou
reviewed
Nov 19, 2024
Co-authored-by: kyri-petrou <67301607+kyri-petrou@users.noreply.github.com>
Contributor
Author
|
@kyri-petrou Done with the changes! |
kyri-petrou
approved these changes
Nov 21, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updated onDone and onDoneCause to execute callbacks synchronously on the main fiber to align behavior with other onX methods removing implicit forking( (i.e. NOT fork).
Closes #9191
/claim #9191
Important Note about Changes to
onDoneandonDoneCauseBehaviorIn this release, we have updated the behavior of
onDoneandonDoneCauseto address concerns raised in Issue #9191. Previously, these methods operated asynchronously by forking their callback effects, which could lead to subtle bugs and race conditions. This behavior was unique among ZIO'sonXmethods and diverged from typical expectations.Key Behavior Change: Synchronous Execution
Starting in this release,
onDoneandonDoneCausenow execute success and failure callbacks synchronously within the calling fiber. This update aligns them with otheronXmethods (e.g.,onExit), ensuring that the effect completes fully, including any specified callbacks, before moving on. This change provides greater predictability and reliability by removing the previously implicit asynchronous behavior.Summary and Impact
This enhancement addresses the risk identified in Issue #9191, where the unexpected forking could cause surprising, hard-to-trace bugs. As a result, users may notice different callback behaviors following this upgrade. Where necessary, alternative patterns (such as explicit forking) can be employed to replicate prior behavior. We encourage users to review any code that relies on
onDoneoronDoneCauseand verify that the updated synchronous behavior aligns with expected outcomes.