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

Bug in Task implementation ? #409

Closed
Gozala opened this Issue Sep 16, 2015 · 1 comment

Comments

Projects
None yet
2 participants
@Gozala

Gozala commented Sep 16, 2015

I have being trying to figure out how Tasks in Elm actually work and either I'm misunderstanding something crucial or I found a bug in the implementation. Relevant lines of code are following:
https://github.com/elm-lang/core/blob/6b505d4f5219b6e45967cc87cf99729bc3f12f03/src/Native/Task.js#L140-L160

So if Async task turns out to actually be sync then step of the running root task steps out into a done state which I believe is incorrect as it assumes that result passed into a callback is either success or failure task:
https://github.com/elm-lang/core/blob/6b505d4f5219b6e45967cc87cf99729bc3f12f03/src/Native/Task.js#L146

On the other hand if task turns out to be async then root is resumed with a result passed into a callback:
https://github.com/elm-lang/core/blob/6b505d4f5219b6e45967cc87cf99729bc3f12f03/src/Native/Task.js#L147-L148
https://github.com/elm-lang/core/blob/6b505d4f5219b6e45967cc87cf99729bc3f12f03/src/Native/Task.js#L155

There for if passed result is neither succeed or fail task routine will be resumed and will continue by running a result task.

From above I conclude that one of following is true:

  1. Running root task may complete pre-maturely (without completing running all the steps) if one of the steps was an 'Async' task, but in reality was sync.
  2. Async tasks are assumed to invoke a callback with either success(v) or failure(v) but that is not verified or enforced in any way. In this case it maybe worth changing an Task.asyncFunction interface so that user is unable to complete with a different task type.
@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Sep 16, 2015

Member

We talked about this offline, but here's the result so we can close this. Case 2 is true. This API is not intended for public consumption so I "do the right thing" when I use the relevant functions. It is conceivable that these things will change if we add some message passing concurrency or open things up more.

Member

evancz commented Sep 16, 2015

We talked about this offline, but here's the result so we can close this. Case 2 is true. This API is not intended for public consumption so I "do the right thing" when I use the relevant functions. It is conceivable that these things will change if we add some message passing concurrency or open things up more.

@evancz evancz closed this Sep 16, 2015

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