Restart: Cleanup kernel process

[lgeiger]

Follow up on #853

This will ensure to kill the old kernel process before starting a new one.

For me this fixes the issue describe by @t9md: #863 (comment)

Unfortunately I can't reproduce #863 so I don't know for sure if this will fix the original issue.

[arl-o]

#863 is still active.

[lgeiger]

@wowserx Just to make sure, have you tested with the latest master? v1.17.0 doesn't yet include this fix.

[arl-o]

I feel silly, wasn't running atom in dev mode.

#863 is no longer active! It works fine.

[lgeiger]

Awesome thanks for checking! 🎉

I'll ship a patch release since this fixes a critical issue.

[t9md]

For me this fixes the issue describe by @t9md: #863 (comment)

I still can reproduce with latest master v1.17.1.

My steps to reproduce is

1. prepare sample code below.

let a = [1, 2, 3]
a.push(10)
a // %%
a.push(20)
a // %%
a.push(30)
a.map(e => e * e)
a // %%

2. execute restart-kernel-and-re-evaluate-bubbles via keymap so that I can execute multiple times with short gab between each execution.

My keymap is here,

'atom-text-editor':
  'cmd-h': 'hydrogen:restart-kernel-and-re-evaluate-bubbles'

3. If I execute ten times(repeat cmd-h ten times while seeing result of each fire), I can see some instance where bubbles are not filled with value(checkmark which represent undefined).

[arl-o]

I can reproduce the above, but it's not the same problem as #863, since the code is definitely running, just not consistently re-populating the bubbles.

In #863, the process was hanging indefinitely, but in the issue described above I've had it work where only some of the bubbles populate, but their results are correct:

I could also reproduce this problem by mashing a custom keymap which calls restart kernel and clear bubbles simultaneously, then quickly running the code.

[BenRussert]

@t9md want to try that my fork on #876? I think this could be related to #870

[t9md]

@BenRussert Sorry for late response(I didn't noticed this comment).
I tried with #876, and but no luck, I can still reproduce this the issue I described above.
How about you?
The step is very simple, "Invoking restart-and-re-evaluate via keymap multiple times for ijavascript kernel"

[BenRussert]

@t9md I see your issue now. This is a weird one.

[t9md]

So you could reproduce it right?

[lgeiger]

Looks like we run into #591 with IJavascript

[lgeiger]

@t9md Is this issue something that comes up in your normal workflow, or does it only happen when you rapidly run this command?

[t9md]

Yes, I can say it's easily happens in normal workflow.
I believe, you and all other people can easily reproduce this issue, by

• invoke restart-kernel-and-re-evaluate-bubbles via keymap on ijavascript kernel.

This is NOT corner case of usage.
As my understanding, I didn't see this issue before hydrogen introduce rapid restart mechanism.
I prefer "slow but reliable" behavior than "quick but occasional un-reliable" behavior.
I want you and other maintainer try to reproduce this issue, and if it's really easy to reproduce as I explained, then my opinion is, we should revert to slow-but-reliable behavior worked in previous version.
I wonder if #591 is related(this is old issue), why this issue was not happened(I believe so) in previous slow restart then begin to appear from rapid restart?

[lgeiger]

👍 I'll take a look at it later this week.

[rgbkrk]

we should revert to slow-but-reliable behavior worked in previous version

That sounds good to me.