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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

console: Fix crash if debug console disabled #555

Merged

Conversation

jodh-intel
Copy link
Contributor

The logic for the debug console meant that if the debug console was disabled, the agent was guaranteed to crash on function exit due to the unsafe code block. Fixed by simplifying the code to use the standard Option idiom for optional values.

Fixes: #554.

Signed-off-by: James O. D. Hunt james.o.hunt@intel.com


Note: This PR includes a few other fixes which are part of #411. However, the tracing PR is still not ready and since these fixes are not actual trace changes, I'd like to land them before yet-more rebase hell caused by tracking the branch head 馃槃

Print a simple version string rather than delaying the output
to display a structured version string. The structured output
is potentially more useful but:

- This output is not consistent with other components.

- Delaying the output makes `--version` unusable in some
  environments (since a lot of setup is called before the
  version string can be output).

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Add the configuration details in the announcement log call.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Move the sandbox creation into a new function.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Remove confusing commented out code and some stray testing code.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
Don't create a thread to wait for the ttRPC server to end - it isn't
required as the operation should be blocked on.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
The logic for the debug console meant that if the debug console was
_disabled_, the agent was guaranteed to crash on function exit due to
the unsafe code block. Fixed by simplifying the code to use the standard
`Option` idiom for optional values.

Fixes: kata-containers#554.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
@jodh-intel jodh-intel force-pushed the 2.0-dev-agent-fix-shutdown-crash branch from cd64544 to d12f920 Compare August 21, 2020 16:53
@jodh-intel
Copy link
Contributor Author

/test

let _ = server.start().unwrap();

handle.join().unwrap();
let _ = rx.recv().map_err(|e| format!("{:?}", e));
Copy link
Member

Choose a reason for hiding this comment

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

@lifupan @yyyeerbo Please take a look in case there were some reason for the waiting thread that we fail to understand now.

Copy link
Member

@lifupan lifupan Aug 26, 2020

Choose a reason for hiding this comment

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

It's really unnecessary, I think it's inherited from the init debug code.

@lifupan lifupan merged commit 6f2eab4 into kata-containers:2.0-dev Aug 26, 2020
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.

Fix crash if debug console disabled
4 participants