-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Bump similar_asserts to 1.6 #4269
Conversation
This allows us to remove a call to `String::leak` as they relaxed the life time requirements on the one method we use.
4547bde
to
325ffc5
Compare
591dc12
to
52dc16f
Compare
52dc16f
to
33ff9ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but it would be better if we could test for a condition rather than blindly wait 100ms. Fine if this is the pragmatic solution for now, but we should improve if we can.
@@ -636,6 +637,9 @@ mod tests { | |||
assert_eq!(release_count.load(Ordering::Relaxed), 2); | |||
assert_eq!(checkin_count.load(Ordering::Relaxed), 3); | |||
assert_eq!(checkout_count.load(Ordering::Relaxed), 3); | |||
// this is required to workaround a segfault while shutting down | |||
// the pool | |||
std::thread::sleep(Duration::from_millis(100)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unfortunate. Do we know what we're waiting for? A timed delay like this has the potential to be unreliable, e.g., if the test system is slow for some reason. If possible it would be best to wait in a loop, testing for whatever condition is needed to avoid the segfault.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct that this is not great. Unfortunately I do not know what exactly we are waiting for, just that waiting seems to resolve the segfaults. My guess is that there is something happening in parallel here that causes the segfault. R2d2 is acquiring a new connection, while the pool is shutting down it might just be that openssl cannot handle creating/dropping a connection in a fast turnaround? Or that it returns early but does stuff on it's own in the background, which in turn leads to collisions while shutting down the connection?
Given that I did not found a way to reproduce the problem locally it's relatively hard for me to debug it. I just went for the first thing that seem to resolve the issue to unblock the other PR's for now. If someone has a better solution I'm more than happy to accept that.
This allows us to remove a call to
String::leak
as they relaxed the life time requirements on the one method we use.