the async API breaks process.nextTick() within the callback

[bman654]

I discovered this issue when I wrapped eval(expr,cb) into a Promise-returning API. Promises execute their callbacks via process.nextTick()

If you call process.nextTick(foo) from within the evaluation callback, then foo will not execute until something else happens (such as another timeout expires).

See these Mocha tests:

import * as julia from 'node-julia';
import { expect } from 'chai';

describe('node-julia', () => {
    it('should work asynchronously', done => {
        julia.eval("2+3", (err, result) => {
            if (!err) {
                expect(result).to.equal(5);
            }
            done(err);
        });
    });

    it('should not mess up process.nextTick() when called asynchronously', done => {
        julia.eval("2+3", (err, result) => {
            if (!err) {
                expect(result).to.equal(5);
            }

            process.nextTick(() => done(err));
        });
    });

    it('should not require calling setTimeout() to fix process.nextTick() when called asynchronously', done => {
        julia.eval("2+3", (err, result) => {
            if (!err) {
                expect(result).to.equal(5);
            }

            process.nextTick(() => done(err));
            setTimeout(() => {}, 0);
        });
    });
});

The first test passes. The second test fails (times out) The third test (where I added an empty setTimeout) passes.

It seems like it is related to this issue: https://github.com/joyent/node/issues/7714

I'm guessing your callbacks are somehow bypassing the nodejs event loop and so the event loop does not know to drain the nextTick queue until something else happens to wake up the event loop.

[bman654]

FYI this is the JavaScript stack trace I get if I print it via (console.log(new Error().stack)).

Inside my callback:

at /.../node-julia-test.es6:17:36

Inside my nextTick callback:

at /.../node-julia-test.es6:28:61
at process._tickCallback (node.js:355:11)

[bman654]

node-julia version: node-julia@1.1.2 (git://github.com/wateim/node-julia.git#73eb78f3873eeaafa96e4c81b7b5560f30e35435) node version: v0.12.5 OS: Linux ubuntu-14 3.13.0-24-generic #46-Ubuntu SMP Thu Apr 10 19:11:08 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

[sebastiang]

and if it helps, https://github.com/JuliaLang/julia/commit/fe7203e8b747a7caf472b59e5fa41f6cc86b1489

[waTeim]

I don't remember doing anything non standard as far as async notification. Just basic uv_async_send, but I do remember there can be only 1 notify delivery for multiple things in uv_async_send so knowing that there's a similar thing that happens in nj maybe something subtle about that. This might prove difficult as inserting logging will alter the eventing behavior. Might be worth attempting with iojs just for perspective and (lack of) reproducibility. BTW just for info, are you using Q or bluebird or something else for the promises wrapping?

[sebastiang]

I'm guessing that it may not be enough to notify libuv about your callback, but you may need to specifically kick in nodejs's bookkeeping, e.g. whatever's going on with process._tickCallback

[bman654]

It looks like one way or another you need to call MakeCallback() to actually run the callback and allow nodejs to trigger its nextTick processing logic (and also its domain logic which I also notice is not working with node-julia.

It looks like all of the node native libraries use this method. Here's the timers, for example: https://github.com/joyent/node/blob/master/src/timer_wrap.cc#L140

FYI, here's some unit tests showing the callbacks also are not running on the current domain:

    it('this one works', done => {
        const d = domain.create();
        d.on('error', () => done());
        d.run(() => {
            process.nextTick(() => { throw new Error("some error"); });
        });
    });
    it('this one fails', done => {
        const d = domain.create();
        d.on('error', () => done());
        d.run(() => {
            julia.eval("2+3", () => { throw new Error("some error"); });
        });
    });

[bman654]

Re: promises. We use RxJs for most of our async stuff so I chose not to pull in a library like bluebird or Q.

So I just use this function to wrap:

/**
 * Converts a Node.js callback style function to a Promise.  This must be in function (err, ...) format.
 * @param {Function} func The function to call
 * @param {Function} [selector] A selector which takes the arguments from the callback minus the error to produce a single item to yield on next.
 * @returns {Function} An async function which when applied, returns a Promise with the callback results (if there are multiple results, then they are supplied as an array)
 */
export function fromNodeCallback(func, selector) {
    var newFunc = function (...args) {
        return new Promise((resolve, reject) => {
            func(...args, (err, ...results) => {
                if (err) {
                    reject(err);
                }
                else {
                    let finalResults = results.length > 1 ? results : results[0];
                    // run the selector if provided
                    if (selector) {
                        try {
                            finalResults = selector(finalResults);
                        }
                        catch (e) {
                            reject(e);
                        }
                    }
                    resolve(finalResults);
                }
            });
        });
    };

    newFunc.displayName = func.displayName;
    return newFunc;
};

Usage:

const evalAsync = fromNodeCallback(julia.eval);
const myPromise = evalAsync("2+3");

[waTeim]

Hey sorry for the catch up questions. Are you guys using a transpiler like traceur or babel, I was able to get mocha to process the test but only after changing back to the using require rather than import.

[bman654]

yes I'm using babel + babel runtime + webpack.

[waTeim]

I believe a7cbe3b2 has addressed this problem, the test case passes, and according to nodejs/nan#284 and this stackoverflow question, it's the right approach.

[bman654]

Yes this checkin resolves the nextTick issue. The domain test still fails. I'll open a different issue for that.

Thanks