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

Stop file system watchers on application quit to prevent exit code !== 0 #7504

Merged

Conversation

jansav
Copy link
Contributor

@jansav jansav commented Apr 5, 2023

On application quit, the watchers should be stopped to ensure clean exit. Previously the application quit resulted in exit code 134. Before #3465, the exit code remained 0 but the watchers weren't cleaned explicitly.

@jansav jansav added the bug Something isn't working label Apr 5, 2023
@jansav jansav added this to the 6.4.15 milestone Apr 5, 2023
@jansav jansav requested a review from a team as a code owner April 5, 2023 09:47
@jansav jansav requested review from jim-docker and gabriel-mirantis and removed request for a team April 5, 2023 09:47
Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>
Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>
@jansav jansav force-pushed the fix/file-system-watchers-causing-crash-on-quit branch from 4ea8042 to deb65d4 Compare April 5, 2023 10:34
@Nokel81 Nokel81 linked an issue Apr 5, 2023 that may be closed by this pull request
@Nokel81 Nokel81 merged commit 696c026 into release/v6.4 Apr 5, 2023
13 checks passed
@Nokel81 Nokel81 deleted the fix/file-system-watchers-causing-crash-on-quit branch April 5, 2023 13:34
Copy link
Contributor

@jim-docker jim-docker left a comment

Choose a reason for hiding this comment

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

userCreateResourceTemplatesInjectable has a watch that doesn't appear to get closed but I can't reproduce the bug even when adding/removing template files

@Nokel81
Copy link
Collaborator

Nokel81 commented Apr 5, 2023

Maybe that is the last watcher and which allows the application to close normally.

Good find, can you please open a PR to explicitly close that watcher too?

@jim-docker
Copy link
Contributor

Doesn't it close normally after this PR?

@Nokel81
Copy link
Collaborator

Nokel81 commented Apr 5, 2023

This PR looks like it closes the kubeconfig watches and the watching of extensions.

I would say that we should close all watcher explicitly, just to be safe.

@jim-docker
Copy link
Contributor

Agreed, just wanted to confirm you weren't still seeing the error.

Maybe that is the last watcher and which allows the application to close normally.

Nokel81 pushed a commit that referenced this pull request Apr 5, 2023
…= 0 (#7504)

* fix: Dispose the kubeconfig watcher when application quits

Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>

* fix: Dispose the extension watcher when application quits

Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>

---------

Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>
Nokel81 pushed a commit that referenced this pull request Apr 5, 2023
…= 0 (#7504)

* fix: Dispose the kubeconfig watcher when application quits

Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>

* fix: Dispose the extension watcher when application quits

Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>

---------

Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>
Nokel81 added a commit that referenced this pull request Apr 5, 2023
Signed-off-by: Sebastian Malton <sebastian@malton.name>
Nokel81 added a commit that referenced this pull request Apr 5, 2023
Signed-off-by: Sebastian Malton <sebastian@malton.name>
@Nokel81 Nokel81 mentioned this pull request Apr 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exit code is not 0 during normal quit
3 participants