Skip to content

childprocess.spawn breaks process.context #1807

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

Closed
barocsi opened this issue Nov 11, 2015 · 5 comments
Closed

childprocess.spawn breaks process.context #1807

barocsi opened this issue Nov 11, 2015 · 5 comments
Assignees
Labels

Comments

@barocsi
Copy link

barocsi commented Nov 11, 2015

Interestingly after calling an imagemagick function from a models remotemethod, after successful callback loopback.getCurrentContext() returns null.

Imagemagick spawns a child process and an event listener does the callback function call.

from imagemagick.js

 var Accumulator = function(cb) {
    this.stdout = {contents: ""};
    this.stderr = {contents: ""};
    this.callback = cb;
    var limitedWrite = function(stream) {
      return function(chunk) {
        stream.contents += chunk;
        if (!killed && stream.contents.length > options.maxBuffer) {
          child.kill(options.killSignal);
          killed = true;
        }
      };
    };
    this.out = limitedWrite(this.stdout);
    this.err = limitedWrite(this.stderr);
  };

Accumulator.prototype.finish = function(err) {
    this.callback(err, this.stdout.contents, this.stderr.contents); 
};

 child.addListener(version[0] == 0 && version[1] < 7 ? "exit" : "close", function (code, signal) {
    console.log("acccc")
    var loopback = require('loopback')
    console.log(loopback.getCurrentContext())
    if (timeoutId) clearTimeout(timeoutId);
    if (code === 0 && signal === null) {
      std.finish(null);
    } else {
      var e = new Error("Command "+(timedOut ? "timed out" : "failed")+": " + std.errCurrent());
      e.timedOut = timedOut;
      e.killed = killed;
      e.code = code;
      e.signal = signal;
      std.finish(e);
    }

where std.finish() simply does the callback

Well I am not sure where is this relates to the getCurrentContext, however in the loopback source there are corresponding lines from current-context.js

loopback.createContext = function(scopeName) {
    // Make the namespace globally visible via the process.context property
    process.context = process.context || {};
    var ns = process.context[scopeName];
    if (!ns) {
      ns = cls.createNamespace(scopeName);
      process.context[scopeName] = ns;
      // Set up loopback.getCurrentContext()
      loopback.getCurrentContext = function() {
        return ns && ns.active ? ns : null;
      };

      chain(juggler);
      chain(remoting);
    }
    return ns;
  };

It seems that loopback simply stores values in the process.context
And more interestingly BEFORE calling the child.spawn command we have a different context object after the child finishes and exits.

Before the call we have a process.context object like this:

{ loopback:
   Namespace {
     name: 'loopback',
     active: { accessToken: [Object], currentUser: [Object] },
     _set: [ null, [Object] ],
     id:
      AsyncListener {
        create: [Function],
        flags: 15,
        before: [Function],
        after: [Function],
        error: [Function],
        uid: 2,
        data: null } } }

but after the child process exits the process.context changes to:

{ loopback:
   Namespace {
     name: 'loopback',
     active: null,
     _set: [],
     id:
      AsyncListener {
        create: [Function],
        flags: 15,
        before: [Function],
        after: [Function],
        error: [Function],
        uid: 2,
        data: null } } }

Any ideas on what could break the context object?

@richardpringle
Copy link
Contributor

@bajtos, do you have any ideas about this?

@bajtos
Copy link
Member

bajtos commented Jan 28, 2016

Not really. The cls module we use for "current context" is brittle and can get easily broken, I'll assume childprocess.spawn is just another example of that. What we can do: write a small standalone program using cls and childprocess.spawn and check whether we can reproduce the issue without loopback. If we can, then the problem should be reported to cls.

@barocsi
Copy link
Author

barocsi commented Feb 2, 2016

Meanwhile any ideas for stable workaround?

@bajtos
Copy link
Member

bajtos commented Oct 7, 2016

Meanwhile any ideas for stable workaround?

You can bind your callback to the correct current context, as is proposed in loopbackio/loopback-connector#56. That way the current context will be correctly restored after your child process finishes.

@bajtos
Copy link
Member

bajtos commented Mar 3, 2017

You can also try the new additions in loopback-context, e.g. https://github.com/strongloop/loopback-context#bind-for-concurrency

We have removed getCurrentContext APIs in LoopBack 3.0, we are not going to further investigate this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants