[daemon_base] fix to not reregister signal handler #4998
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
-src/sonic-daemon-base/sonic_daemon_base/daemon_base.py Problem: Currently all daemons inherit from daemon_base class, and for signal handling functionality they register the signal_handler() by overriding the siganl_handler() in daemon_base by their own implmentation. But some sonic_platform instances also can invoke the daemon_base constructor while trying to instantiate the common utilities for example platform_chassis = sonic_platform.platform.Platform().get_chassis() This will cause the re registration of signal_handler which will cause base class signal_handler() to be invoked when the daemon gets a signal, whereas their own signal_handler should have been invoked. Fix: We only register the siganl_handler once, and if signal_handler has been registered, not re register it. Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
jleveque
suggested changes
Jul 17, 2020
added a coment for the changes Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
jleveque
suggested changes
Jul 18, 2020
fixing some spellings Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
jleveque
approved these changes
Jul 18, 2020
|
Retest vsimage please |
1 similar comment
|
Retest vsimage please |
|
Just a note for posterity: only Python-based daemons should inherit from the DaemonBase class or even import daemon_base.py. However, currently, other Python code is using this module, like the platform API, which in turn has created the situation which is being fixed by this PR. I have intended to create a Python library that both DaemonBase as well as non-daemon code can import which contains common, general SONiC-related functions. I have opened an issue to track this: #4999 |
|
retest vsimage please |
nazariig
approved these changes
Jul 18, 2020
abdosi
pushed a commit
that referenced
this pull request
Jul 26, 2020
* [daemon_base] fix to not reregister signal handler -src/sonic-daemon-base/sonic_daemon_base/daemon_base.py Problem: Currently all daemons inherit from daemon_base class, and for signal handling functionality they register the signal_handler() by overriding the siganl_handler() in daemon_base by their own implmentation. But some sonic_platform instances also can invoke the daemon_base constructor while trying to instantiate the common utilities for example platform_chassis = sonic_platform.platform.Platform().get_chassis() This will cause the re registration of signal_handler which will cause base class signal_handler() to be invoked when the daemon gets a signal, whereas their own signal_handler should have been invoked. Fix: We only register the siganl_handler once, and if signal_handler has been registered, not re register it. Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com> * [daemon_base] fix to not reregister signal handler Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why I did it
Currently all daemons inherit from daemon_base class, and for
signal handling functionality they register the signal_handler() by
overriding the siganl_handler() in daemon_base by their own
implmentation.
But some sonic_platform instances also can invoke the daemon_base
constructor while trying to instantiate the common utilities
for example
platform_chassis = sonic_platform.platform.Platform().get_chassis()
This will cause the re registration of signal_handler which will
cause base class signal_handler() to be invoked when the daemon
gets a signal, whereas their own signal_handler should have been
invoked.
How I did it
We only register the siganl_handler once, and if signal_handler has
been registered, not re register it.
How to verify it
Run any daemon and send a signal to it and ensure that the
correct signal_handler is invoked.
-src/sonic-daemon-base/sonic_daemon_base/daemon_base.py
added an if check for signal registration to not re register if
the signal is already registered.
Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com