Skip to content

Logger thread spins forever if there is no backend #10054

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

Closed
aescolar opened this issue Sep 18, 2018 · 3 comments
Closed

Logger thread spins forever if there is no backend #10054

aescolar opened this issue Sep 18, 2018 · 3 comments
Assignees
Labels
area: Logging bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@aescolar
Copy link
Member

aescolar commented Sep 18, 2018

A configuration with no backend registered will have the logger thread spin forever.

Instead having an assert at the start of the thread would be sensible, something like this: fb0e038

As of now there is 2 Xtensa tests in CI with this issue.

@aescolar aescolar added bug The issue is a bug, or the PR is fixing a bug area: Logging labels Sep 18, 2018
@nashif nashif added the priority: medium Medium impact/importance bug label Sep 30, 2018
@nordic-krch
Copy link
Collaborator

In case no backends are present in the system we can add assert since in that case it's better to disable logger. What are the issues with xtensa tests? Will it fix it?

@aescolar
Copy link
Member Author

In case no backends are present in the system we can add assert since in that case it's better to disable logger.

Agreed.
Having an assert would be also a nice way of pointing to the reason of the (configured in) logger not working (there was no backends for it).

What are the issues with xtensa tests?

That the assert will trigger for them, (assuming they haven't been fixed yet) and they will fail in CI.
Meaning, the assert will work as expected.

Will it fix it?

An assert does not fix things. It fails noisily when the error condition happens.

@nordic-krch
Copy link
Collaborator

added assert in #10513, if CI fails i'll go from there.

@nashif nashif added the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label Mar 4, 2019
@ghost ghost removed the In progress For PRs: is work in progress and should not be merged yet. For issues: Is being worked on label Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Logging bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Development

No branches or pull requests

3 participants