async callbacks are not capturing the active domain

[bman654]

If using nodejs domains, then async callbacks should run in the context of the current domain.

This doesn't appear to be happening with node-julia.

Here's some unit tests.
The first one fails.
The 2nd one (passes) shows how it should work with an example from setTimeout.
The 3rd one (passes) shows how if I manually bind my callback to the domain then it works.
The 4th one (fails) shows domain.active is not set correctly
The 5th & 6th one show that it is set correctly when using setTimeout or domain.bind:

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

describe('it should work with domains', () => {

    it('should throw errors to the domain context', done => {
        const d = domain.create();
        d.on('error', () => done());
        d.run(() => julia.eval("2+3", () => { throw new Error("some error"); }));
    });

    it('should throw errors to the domain context with d.bind()', done => {
        const d = domain.create();
        d.on('error', () => done());
        julia.eval("2+3", d.bind(() => { throw new Error("some error"); }));
    });

    it('should set active domain inside timeout', done => {
        const d = domain.create();
        d.on('error', done);
        d.run(() => setTimeout(() => {
            expect(domain.active).to.equal(d);
            done();
        }, 0));
    });

    it('should set active domain inside callback', done => {
        const d = domain.create();
        d.on('error', done);
        d.run(() => julia.eval("2+3", () => {
            expect(domain.active).to.equal(d);
            done();
        }));
    });

    it('should set active domain inside callback when using bind()', done => {
        const d = domain.create();
        d.on('error', done);
        julia.eval("2+3", d.bind(() => {
            expect(domain.active).to.equal(d);
            done();
        }));
    });
});

It looks like the current domain (Context?) is not being captured when the work is queued?

[waTeim]

That is certainly true and unless it's implicitly included in the Scope/Closure, then the only thing that prevents the synchronous case from failing as well is that it's throws to the current context which is connected to the domain. I feel this one should also be a result of an oversight and will be straightforward to address.

[waTeim]

I've looked at the various blogs/documentation about how this is supposed to work, and supposedly it's all happen automatically so long as MakeCallback is used, but of course, that's not what's happening. So I'm going to have to fix this because it's obviously not working like it's supposed to but it will need to be a bug fix to 1.2.

[bman654]

I suspect it is the first argument to MakeCallback. You call some function to get the global context at the time you are calling back and so might be capturing the wrong context. I wonder if you should call that code to get the global context when queuing the request and then storing that captured context with the callback so you can pass it to MakeCallback later. That way you would capture the context in place when the call to julia.eval is made.

Brandon

On Jul 7, 2015, at 9:40 PM, Jeff Waller [email protected] wrote:

I've looked at the various blogs/documentation about how this is supposed to work, and supposedly it's all happen automatically so long as MakeCallback is used, but of course, that's not what's happening. So I'm going to have to fix this because it's obviously not working like it's supposed to but it will need to be a bug fix to 1.2.

—
Reply to this email directly or view it on GitHub.

[waTeim]

That's possible. However, this slideware especially slide 14 and in the node src here and here, this appears to more closely associated with the Environment (node internal) and derived from the 2nd parameter.

Indeed that 2nd parameter is set to a global value which is likely wrong here

What it should be set to instead, though... Ah, maybe it's args.This() from the original call. but for a bare function, I felt that would be set to null. No, wait maybe it's the closure object... I'm going to at least try that.

Update: yes args.This() is null. in julia.eval.

[bman654]

ah yes it is the 2nd argument I meant to say. Maybe you need to capture that global variable value at the start of eval() in order to capture the"current environment" for the callback.

Brandon

On Jul 7, 2015, at 10:11 PM, Jeff Waller [email protected] wrote:

That's possible. However, this slideware especially slide 14 and in the node src here and here, this appears to more closely associated with the Environment (node internal) and derived from the 2nd parameter.

Indeed that 2nd parameter is set to a global value which is likely wrong here

What it should be set to instead, though... Ah, maybe it's args.This() from the original call. but for a bare function, I felt that would be set to null. No, wait maybe it's the closure object... I'm going to at least try that.

—
Reply to this email directly or view it on GitHub.

[waTeim]

Attempted that, and various other things but so far have not obtained the desired result. I feel that's very close to what needs to happen however.

[waTeim]

This proved to be harder than expected. args.This() was not actually null, but nevertheless I was not successful in using it. I finally had to give up and ask on the nodejs forum, which turned out great. I think that 7116173 addresses at least the issue for exec and eval.

But, what to do about calling JS-ified Julia structs seems not so straightforward.