Asynchronous loops created with Tasks and Signal.mailbox trigger crash in stepTask

[pchiusano]

Here is a minimal gist demonstrating the problem and showing a workaround. Clicking rapidly generates:

Uncaught TypeError: Cannot read property 'status' of undefined

pointing to this line in Task.js stepTask.

The problematic pattern seems to be signals with 'asynchronous loops', whose output generates asynchronous tasks that affect the future value of that signal. I worked around it with an intermediate 'relay' mailbox, but that seems pretty kludgy.

Background discussion on the mailing list

[srid]

FWIW, I hit this bug in my app and had to workaround it (see the two commits linked above).

[srid]

FWIW, I hit this bug in my app and had to workaround it (see the two commits linked above).

[Dobiasd]

fyi, I also bumped into this problem in one of my apps. Luckily it happens with a task, that does not harm if it fails now and then. :) So I guess I will just leave it and wait for this issue to be fixed.

[Dobiasd]

fyi, I also bumped into this problem in one of my apps. Luckily it happens with a task, that does not harm if it fails now and then. :) So I guess I will just leave it and wait for this issue to be fixed.

[evancz]

If you can take a swing at the fix, it would be very appreciated!

[evancz]

If you can take a swing at the fix, it would be very appreciated!

[Dobiasd]

@evancz I'm note sure if this was for me or not. In case it was: I would love to, but I do not even know where to start. Despite using Elm a lot, I have no clue about how it works internally. And I don't even know how I could start learning about it. :D

[Dobiasd]

@evancz I'm note sure if this was for me or not. In case it was: I would love to, but I do not even know where to start. Despite using Elm a lot, I have no clue about how it works internally. And I don't even know how I could start learning about it. :D

[evancz]

For anyone who can help really! You can mess with it locally by manually modifying elm-stuff/packages/elm-lang/core/Native/Task.js in your project. You can add console.log or whatever else into the JS and that'll get packaged up when you run elm-make. If you can find the error in your version of Native/Task.js that'll be the bug.

[evancz]

For anyone who can help really! You can mess with it locally by manually modifying elm-stuff/packages/elm-lang/core/Native/Task.js in your project. You can add console.log or whatever else into the JS and that'll get packaged up when you run elm-make. If you can find the error in your version of Native/Task.js that'll be the bug.

[colinmccabe]

I'm seeing these exceptions with a Time.every that gets mapped to an Http request whose result is mailed to a view. There is no loop. The exceptions are sporadic and random and happen even when the request succeeds. Oddly, the pace of the exceptions quickens dramatically when I'm in a different browser tab.

I'm rather obsessed with this and want to fix it. Unfortunately I'm clueless about how Elm actually works. I'll do what I can.

[colinmccabe]

I'm seeing these exceptions with a Time.every that gets mapped to an Http request whose result is mailed to a view. There is no loop. The exceptions are sporadic and random and happen even when the request succeeds. Oddly, the pace of the exceptions quickens dramatically when I'm in a different browser tab.

I'm rather obsessed with this and want to fix it. Unfortunately I'm clueless about how Elm actually works. I'll do what I can.

[colinmccabe]

This is what is causing the error in my case:

• A task chain (not sure what to call this) containing an Async task somewhere within it gets passed to the register() function and is placed on the workQueue (Task.js#L83)
• register() calls runTask() which then calls stepTask(), where the Async task gets replaced by an empty placeholder object (Task.js#L140).
• At some point, onComplete() runs (Task.js#L71) and passes the chain to runTask() which in turn calls stepTask(). Most of the time, the empty placeholder object has been replaced by a Success object by the asyncFunction (Task.js#L144) and all is well. Occasionally, an empty placeholder object is there, and stepTask() chokes on it.

One theory is that workQueue[0] gets overwritten by the value of the next Async task before runTask() runs for the preceding Async. This would explain the presence of the placeholder object.

One bit of evidence in favor of this theory is that removing the setTimeout() wrapper around runTask() makes the exception go away, possibly because this gives no time for workQueue[0] to be overwritten.

function onComplete()
{
    workQueue.shift();

    if (workQueue.length > 0)
    {
        runTask(workQueue[0], onComplete);
    }
}

Of course this change probably causes bad effects elsewhere.

[colinmccabe]

This is what is causing the error in my case:

• A task chain (not sure what to call this) containing an Async task somewhere within it gets passed to the register() function and is placed on the workQueue (Task.js#L83)
• register() calls runTask() which then calls stepTask(), where the Async task gets replaced by an empty placeholder object (Task.js#L140).
• At some point, onComplete() runs (Task.js#L71) and passes the chain to runTask() which in turn calls stepTask(). Most of the time, the empty placeholder object has been replaced by a Success object by the asyncFunction (Task.js#L144) and all is well. Occasionally, an empty placeholder object is there, and stepTask() chokes on it.

One theory is that workQueue[0] gets overwritten by the value of the next Async task before runTask() runs for the preceding Async. This would explain the presence of the placeholder object.

One bit of evidence in favor of this theory is that removing the setTimeout() wrapper around runTask() makes the exception go away, possibly because this gives no time for workQueue[0] to be overwritten.

function onComplete()
{
    workQueue.shift();

    if (workQueue.length > 0)
    {
        runTask(workQueue[0], onComplete);
    }
}

Of course this change probably causes bad effects elsewhere.

[jvoigtlaender]

Has #304 completely fixed this? If so, the issue should be closed. Otherwise, what is still problematic?

[jvoigtlaender]

Has #304 completely fixed this? If so, the issue should be closed. Otherwise, what is still problematic?

[jvoigtlaender]

The previous comment is really a question for @colinmccabe just as for anybody else. If you don't have the repository rights to close issues here, I have, but I need confirmation that the issue is really resolved.

[jvoigtlaender]

The previous comment is really a question for @colinmccabe just as for anybody else. If you don't have the repository rights to close issues here, I have, but I need confirmation that the issue is really resolved.

[colinmccabe]

I used "mitigate" because I wasn't sure that the commit would fix it, but I think it does. I haven't seen another runtime exception in my project.

Here is the problematic series of events in the old code:

1. onComplete() is called after a task completes. It shifts the task off the workQueue, which becomes empty.
2. onComplete() calls setTimeout() and returns. Control could shift to anywhere!
3. Control moves to register() because a new task happens to be coming in. The new task is pushed into workQueue[0]. The workQueue is now 1 item long, so register() calls runTask(workQueue[0], ...). runTask() calls stepTask() which replaces async tasks by empty placeHolder objects.
4. Control moves to the code inside the setTimeout() in onComplete(). Since the workQueue is not empty, it calls runTask(workQueue[0], ...) a second time, before the async tasks have a chance complete! The empty placeholder objects cause the runtime exception.

Moving the if check outside of the setTimeout removes the possibility of preemption, ensuring that step 3) can never happen between steps 2) and 4).

If this makes sense, feel free to close.

[colinmccabe]

I used "mitigate" because I wasn't sure that the commit would fix it, but I think it does. I haven't seen another runtime exception in my project.

Here is the problematic series of events in the old code:

1. onComplete() is called after a task completes. It shifts the task off the workQueue, which becomes empty.
2. onComplete() calls setTimeout() and returns. Control could shift to anywhere!
3. Control moves to register() because a new task happens to be coming in. The new task is pushed into workQueue[0]. The workQueue is now 1 item long, so register() calls runTask(workQueue[0], ...). runTask() calls stepTask() which replaces async tasks by empty placeHolder objects.
4. Control moves to the code inside the setTimeout() in onComplete(). Since the workQueue is not empty, it calls runTask(workQueue[0], ...) a second time, before the async tasks have a chance complete! The empty placeholder objects cause the runtime exception.

Moving the if check outside of the setTimeout removes the possibility of preemption, ensuring that step 3) can never happen between steps 2) and 4).

If this makes sense, feel free to close.

[Dobiasd]

I also do not observe these errors any more in my project that previously had them in abundance.

[Dobiasd]

I also do not observe these errors any more in my project that previously had them in abundance.

[jvoigtlaender]

Sounds all good, so I'm closing this issue. Thanks!

[jvoigtlaender]

Sounds all good, so I'm closing this issue. Thanks!