Skip to content
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

fix(launcher): sigkill after sigint if not dead #823

Closed

Conversation

lzatorski
Copy link
Contributor

In some cases chrome on linux does not die when it receives a sigint
signal. So after a sigint is sent wait 2 more seconds and issue a
sigkill if the process is still running.

this.id = id;
this.state = null;
this._tempDir = path.normalize((env.TMPDIR || env.TMP || env.TEMP || '/tmp') + '/karma-' +
id.toString());
this.exitCallbacks = [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make it just a local var (var exitCallbacks)?

And remove the var exitCallback on line 19, as we don't need it anymore...

@vojtajina
Copy link
Contributor

Thanks @lzatorski !

Can you also squash all your changes into a single commit?

Did you sign the Google CLA?
I need you to sign it, before I can merge this PR.

Again, thanks a lot!

commit 76847fe
Author: Lukasz Zatorski <lzatorski@gmail.com>
Date:   Fri Nov 15 13:41:04 2013 -0800

    fix(launcher): behavior fixed for cleaning tmp directory

commit 09dd0d5
Author: Lukasz Zatorski <lzatorski@gmail.com>
Date:   Wed Nov 13 19:31:43 2013 -0800

    fix(launcher): Removed trailing whitespace

commit 9653c85
Author: Lukasz Zatorski <lzatorski@gmail.com>
Date:   Tue Nov 12 19:12:15 2013 -0800

    fix(launcher): sigkill after sigint if not dead

    In some cases chrome on linux does not die when it receives a sigint
    signal. So after a sigint is sent wait 2 more seconds and issue a
    sigkill if the process is still running.
@lzatorski
Copy link
Contributor Author

Hi @vojtajina , I made changes you asked and a fix for correct tmp directory cleaning and squashed all commits together (is it fine?). I signed the Google CLA this morning :)

@vojtajina
Copy link
Contributor

Thanks @lzatorski ! Merged as c0fa49a.

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

Successfully merging this pull request may close these issues.

2 participants