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

remove unwrap from VcRequestSender #2687

Merged

Conversation

kziemianek
Copy link
Member

Prevents from panics if sender is not yet initialized and vc requests are consumed.

@kziemianek kziemianek requested a review from a team April 24, 2024 08:51
Copy link

linear bot commented Apr 24, 2024

@kziemianek kziemianek added D2-bug Something isn't working and removed D2-bug Something isn't working labels Apr 24, 2024
@BillyWooo
Copy link
Collaborator

The fix looks fine to me. But what do you think to revisit the structure/working flow of threads/initialization steps? I think it's better to eliminate it by design.

@kziemianek
Copy link
Member Author

  1. We must eliminate unwraps - otherwise worker is prone to panics
  2. We may optionally improve design

The issue has Urgent priority and S estimation so I believe we aim here only panic as a root cause of a worker failure. Same fix was applied to StfRequestSender before.

I think we can try to improve the design and avoid using globals - this should be addressed separately.

@BillyWooo BillyWooo merged commit f872bf2 into dev Apr 25, 2024
31 checks passed
@BillyWooo BillyWooo deleted the p-703-race-condition-in-global-component-initialisation branch April 25, 2024 09:53
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.

None yet

2 participants