Skip to content

Callback received in Task.create should be thread-safe and should shift back to default Scheduler#917

Merged
alexandru merged 17 commits into
monix:masterfrom
alexandru:issue908-test
Aug 1, 2019
Merged

Callback received in Task.create should be thread-safe and should shift back to default Scheduler#917
alexandru merged 17 commits into
monix:masterfrom
alexandru:issue908-test

Conversation

@alexandru
Copy link
Copy Markdown
Member

@alexandru alexandru commented Jun 20, 2019 •
edited
Loading

Changes in these PR:

  • in attempting to investigate Sporadic scala.MatchError inside Task.timeoutTo #908, it became clear that the Callback passed in Task.async (and variants) should be thread-safe
    • this should be the user's responsibility, but in case the user fails to uphold the contract, it falls on us to investigate potential bugs and it creates frustration for the user
    • in case onSuccess / onError are called twice, an IllegalStateException is still being thrown, the difference is that now this conflict is able to be detected in a multi-threading context
    • this should give us or the users better error messages
  • removing the allowContinueOnCallingThread flag on Task.async, Task.cancelable, Task.create, etc... introduced in Shifting back to the default pool in Task.fromFuture, Task.async and … #921
    • we don't need this flag
    • this is now replaced with a cool optimization that detects if the register function actually forks and if so, then we shift back, if not then we don't shift
  • also changesTask.fromFuture to use Future.onComplete as it was meant to be used

@alexandru
Copy link
Copy Markdown
Member Author

alexandru commented Jun 20, 2019

Released test version: 3.0.0-RC2-cb74787

@alexandru alexandru mentioned this pull request Jun 20, 2019
Open
@alexandru alexandru added this to the 3.0.0 milestone Jul 28, 2019
@alexandru alexandru changed the title WIP: issue #908 test Callback received in Task.async should be thread-safe Jul 28, 2019
@alexandru alexandru changed the title Callback received in Task.async should be thread-safe [WIP] Callback received in Task.create should be thread-safe and should shift back to default Scheduler Jul 28, 2019
@alexandru alexandru changed the title [WIP] Callback received in Task.create should be thread-safe and should shift back to default Scheduler Callback received in Task.create should be thread-safe and should shift back to default Scheduler Jul 28, 2019
Avasil
Avasil reviewed Jul 28, 2019
View reviewed changes
Comment thread project/MimaFilters.scala Outdated
Avasil
Avasil reviewed Jul 28, 2019
View reviewed changes
Comment thread monix-eval/jvm/src/test/scala/monix/eval/TaskAsyncAutoShiftJVMSuite.scala
Comment thread monix-eval/jvm/src/test/scala/monix/eval/TaskAsyncAutoShiftJVMSuite.scala
Comment thread monix-eval/jvm/src/test/scala/monix/eval/TaskAsyncAutoShiftJVMSuite.scala
Comment thread monix-eval/shared/src/main/scala/monix/eval/internal/TaskCreate.scala
Comment thread monix-eval/shared/src/test/scala/monix/eval/TaskAsyncSuite.scala Outdated
Comment thread monix-eval/shared/src/test/scala/monix/eval/TaskCancelableSuite.scala Outdated
Comment thread monix-eval/shared/src/test/scala/monix/eval/TaskFromFutureSuite.scala Outdated
oleg-py
oleg-py reviewed Jul 29, 2019
View reviewed changes
Comment thread monix-eval/shared/src/main/scala/monix/eval/internal/TaskCreate.scala
@alexandru alexandru merged commit a2c66b0 into monix:master Aug 1, 2019
@alexandru alexandru mentioned this pull request Aug 2, 2019
Merged
@Avasil Avasil mentioned this pull request Nov 16, 2019
Merged
mdedetrich pushed a commit to mdedetrich/monix that referenced this pull request Mar 28, 2020
@alexandru @mdedetrich
Callback received in Task.create should be thread-safe and should shi…
de35236 
…ft back to default Scheduler (monix#917)

* Issue monix#908 - test

* Making callback idempotent

* Silence errors

* Small fix

* Fix bug

* Working on tests

* More tests for Task

* Cleanup old tess

* Fix mima

* Formatting

* Cleanup Mima filters

* Revert tests

* Remove IntelliJ junk

* Remove IntelliJ junk

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

Reviewers

@Avasil Avasil Avasil left review comments

+1 more reviewer

@oleg-py oleg-py oleg-py approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@alexandru @oleg-py @Avasil