Skip to content

Change behavior of 1st-resume. #36

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

Merged
merged 2 commits into from
May 23, 2015
Merged

Change behavior of 1st-resume. #36

merged 2 commits into from
May 23, 2015

Conversation

inamiy
Copy link
Member

@inamiy inamiy commented May 23, 2015

Continuing #33 (thread-safety).

This pull request will improve 1st-handleResume() (call of initResumeClosure) by not switching around internal states to force-call configure.resume() when task's initial state is .Paused.
(switching around can be dangerous when transitions e.g. handleProgress() occurs in other threads)

Also, force-calling configure.resume() after initResumeClosure() will be annoying especially in ReactKit when it wants to 1st-resume upstream in other threads (e.g. startAsync or Rx.subscribeOn), whereas configure.resume() will 1st-resume in current thread, so its timing will be indeterminate.

Thus, from ver 3.2.0, configure.resume() will be called from 2nd time of resume(),
considering 1st time as start of the task.
This change will not modify current API, but behavior may change slightly.

NOTE If you are using SwiftTask as standalone, this fix will not affect in most cases because its initial state is set as .Running by default.

inamiy added 2 commits May 21, 2015 15:37
…nitial state so that other transitions e.g. handleProgress can be safely called from other threads.
@inamiy inamiy added this to the 3.2.0 milestone May 23, 2015
inamiy added a commit that referenced this pull request May 23, 2015
@inamiy inamiy merged commit 6744fd6 into master May 23, 2015
@inamiy inamiy deleted the feature/thread-safety branch May 23, 2015 10:58
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.

1 participant