Fix test issues on AIX

[CurryKitten]

This is a fix to support issue #24. I found that looking at AIX, there's an issue when using the restart command in the debugger. AIX seems unable to shutdown it's process quickly enough and still has the socket when the new debugging attempts to bind to port 9229.

In order to fix this, I added a small delay so when the child process is killed it has time to shut down it's socket before restarting.

The other issue that seems unique to AIX involves the issue, once again, of restarting after adding some breakpoints (the preserve-breaks.test.js test) There seems to be an issue here in the ordering of the output from the debugger. Sometimes we see the "breakpoints restored" message before the source listing, and other times it would come afterwards. The side effect of this message appearing at the end like this, is that it would overwrite the "debug>" prompt and then the test would fail.

This might need more investigation to see if the ordering of the text can be fixed, but in order to fix the ci run, I've made a change to explicitly put the "debug>" prompt out after the "breakpoints restored" message is output.

There's one further problem in AIX I observed in which the initial breakpoint in the exceptions.test.js test looks incorrect, but I will open a new issue to track this.

[aqrln]

Couldn't it be

return this.killChild().then(() => {
  return this._runScript();
}).then((child) => {
  // one indentation level less
});

?

[jkrems]

Yeah, nested .thens are a slippery slope. I'd rather flatten this.

[jkrems]

Thanks for looking into these! One minor style nit. CI run: https://ci.nodejs.org/view/x%20-%20Diagnostics/job/node-inspect-continuous-integration/14/

[jkrems]

Yeah, nested .thens are a slippery slope. I'd rather flatten this.

[jkrems]

Woo! AIX passed on CI.

[mhdawson]

I'm wondering if one part of the issue(the "breakpoints restored" printing later than expected) may be related to a race in setting up the breakpoints after a restart. It seems like the setup of the breakpoints may be running in parallel with the code running to hit the first breakpoint. That might also explain .the issue @CurryKitten mentioned in hitting the wrong breakpoint in exceptions.test.js as I think it does a restart as well.

If I added a delay in inspect_repl.js so that restoreBreakpoints() is delayed by 200ms in this part of the code I can get it to fail consistently on OSX in the same way as we see on AIX:

inspector.client.on('ready', function() {
      setTimeout(function() {
        restoreBreakpoints();
        Debugger.setPauseOnExceptions({ state: pauseOnExceptionState });
      }, 200);
    });

since restoreBreakpoints() is asynchronous the is no guarantee how fast it will complete, so adding the delay just opens the existing timing window further.

The code below seemed to fix the preserve-break.tests.js even with a delay inserted (although I did not test on AIX), however, it broke other tests so its obviously not the right way to make sure the other code is not still running in parallel with restoring the breakpoints.

 inspector.client.on('ready', function () {
      Debugger.pause().then(function() {
        restoreBreakpoints();
        Debugger.setPauseOnExceptions({ state: pauseOnExceptionState });
        Debugger.resume();
      });
    });

@jkrems any thoughts ?

[jkrems]

I agree, delaying the ready surfaces the issue more clearly. I'm playing around with that .pause solution - I assume it would require us to ignore pause events somehow (?) because otherwise I'm seeing a "break in bootstrap_node.js" which is kind of confusing. I'm not 100% sure but I suspect the call to Runtime.runIfWaitingForDebugger might be helpful..? Have to do a bit of digging to understand its semantics properly (the naive approach of delaying the call doesn't work because it keeps all others from returning).

[mhdawson]

@jkrems thanks for investigating. I had tried a few options but non seemed right. What we really want is for the application/repl to be paused on startup, we add the breakpoints and then we resume and show the initial debug prompt.

[jkrems]

The next step would be to look under the hood of Chrome Devtools to see in which order they are doing it. Might get around to that tonight.

[jkrems]

I think bundling runIfWaitingForDebugger with restoring the breakpoints works, see #34.

[jkrems]

Thanks for investigating this! I took this fix but changed the fixed timeout to "wait until port is free with timeout". Landed via f6ccfc7.