origamitower / folktale

[not actively maintained!] A standard library for functional programming in JavaScript
https://folktale.origamitower.com/
MIT License
2.04k stars 102 forks source link

Makes race conditions harder with the Task API #122

Closed robotlolita closed 7 years ago

robotlolita commented 7 years ago

Previously the Task API had separate functions for cleanup and for handling cancellation. These functions were defined outside of the Task computation, which means Task had to return references to the allocated resources so those functions could do their jobs.

This obviously didn't work for places where resolution happened synchronously, as they'd have to be called before those references were returned. Calling them asynchronously meant that race conditions on when exactly "cancellation/cleanup" and pending resolutions happen were possible. Which is not a good thing.

Merely making all resolutions asynchronous wouldn't really fix the issue, although it would have made it less likely to happen.

Anyway, I decided to have the computation itself add handlers for cancellation and cleanup events, and those can be just called synchronously, as they'd have access to the resources in the task's scope — testing for the existence of a resource is still required, as cancellation may happen before a particular resource is properly initialised.

What this means is basically:

    // BEFORE
    task(
      resolver => {
        const res = doThings(() => {
          resolver.resolve(value);
        });
        return res;
      },
      res => { cleanup... },
      res => { cancellation... }
    );

    // AFTER
    task(
      resolver => {
        const res = doThings(() => {
          resolver.resolve(value);
        });
        return res;
      }
    );

Cancellation and cleanup handlers must be attached before a task resolves. It's an error to try attaching them after a task is resolved.

This should fix #83… hopefully…

robotlolita commented 7 years ago

Good: this does indeed fix #83.

Bad: IE9~10 somehow have problems with promisedToTask, and IE11 just hangs. Will look into this next week with local IE installs.