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

Fix RpcMonitor init in HealthMonitor #12196

Merged
merged 3 commits into from
Jan 6, 2024

Conversation

ichthus1604
Copy link
Collaborator

Fixes #12066

@molnard molnard self-requested a review January 5, 2024 20:28
Copy link
Collaborator

@molnard molnard left a comment

Choose a reason for hiding this comment

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

tACK - please review my additions.

@molnard
Copy link
Collaborator

molnard commented Jan 5, 2024

If anyone is reviewing this. The solution might seem to be a hack. But the UI needs to be displayed and some services initializes later and that is a legitimate situation. What should the UI do if the service is not ready or not even exist? The question answered here by a loop and null check. This is fixing the issue.

I checked alternate solutions on the business side and got more ugly solutions.

Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

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

tACK

@ichthus1604
Copy link
Collaborator Author

ichthus1604 commented Jan 6, 2024

@molnard A more nuanced and general solution to this problem is to wrap the services in a monad (Task). Then the UI code could request them like this:

var someService = await Services.GetAsync<SomeService>();

which would await until the service is ready. That would remove the endless loop (which I got to admit is a little hacky). Only downside to this approach is that you can't await inside a constructor, and many of the Model classes in the UI are doing just that: referencing services in their constructor and performing event handling on them.

If we moved more of the UI code to asyncs instead of ctors, we could further defer the dependencies from the UI code to these services, which means we could start the UI earlier.

@molnard
Copy link
Collaborator

molnard commented Jan 6, 2024

which means we could start the UI earlier

That is a goal I would like us to achieve until the next release. I noticed lately with the releases that the Wasabi window pops up slowly. It is almost 10 seconds on my laptop.

@molnard molnard merged commit 69ca748 into WalletWasabi:master Jan 6, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Full node is unresponsive while using the built in Knots
3 participants