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

The initialization order of EventServiceManager and persistent_data::ConfigFile does not appear to be guaranteed. #299

Open
4 tasks done
lijl44 opened this issue Jan 24, 2025 · 4 comments

Comments

@lijl44
Copy link

lijl44 commented Jan 24, 2025

Is this the right place to submit this?

  • This is not a security vulnerability or a crashing bug
  • This is not a question about how to use OpenBMC
  • This is not a bug in an OpenBMC fork or a bug in code still under code review.
  • This is not a request for a new feature.

Bug Description

Image

EventServiceManager is initialized at construct time to read persistent data from EventServiceStore. But the data in EventServiceStore is written by persistent_data::ConfigFile at initialization time.

So I think persistent_data::ConfigFile should be initialized earlier than EventServiceManager.

bmcweb initializes EventServiceManager very early in the run() function. but I didn't find code to explicitly initialize persistent_data::ConfigFile before EventServiceManager initialization, which would cause bmcweb to lose the event configuration. (maybe I didn't find the relevant code)

Could you help confirm this? Thanks.

Version

4c8c12d2a699641ec24a2605583686a03a8484cb

Additional Information

No response

@edtanous
Copy link
Contributor

Does this behavior cause an actual functional issue in the image? If so, how do you reproduce it?

ConfigFile class is initialized in the constructor, here:

So it's not clear to me what a fix would even look like.

@lijl44
Copy link
Author

lijl44 commented Feb 5, 2025

In short, I think I need to call persistent_data::getConfig(); explicitly after loadOldBehavior(); to initialize EventServiceStore, otherwise it will lead to loss of subscription information after restarting bmcweb.

@edtanous
Copy link
Contributor

edtanous commented Feb 5, 2025

lead to loss of subscription information after restarting bmcweb.

Can you write this down to a set of steps to reproduce the issue?

@edtanous
Copy link
Contributor

edtanous commented Mar 1, 2025

@lijl44 any updates here?

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

No branches or pull requests

2 participants