Skip to content

Fix CURL filesystem deadlock with corporate proxy#16499

Closed
SoftwareApe wants to merge 1 commit intomicrosoft:masterfrom
SoftwareApe:fix_curl_behind_proxy_filesystem_deadlock_16476
Closed

Fix CURL filesystem deadlock with corporate proxy#16499
SoftwareApe wants to merge 1 commit intomicrosoft:masterfrom
SoftwareApe:fix_curl_behind_proxy_filesystem_deadlock_16476

Conversation

@SoftwareApe
Copy link
Copy Markdown
Contributor

The CURL call to the metrics endpoint doesn't have a timeout. When executing this behind a corporate proxy that blocks this access the filesystem is in a deadlock until curl is killed. By adding a 200ms timeout we give ample time for CURL to finish its request, while also not stopping usage inside local network.

Describe the pull request

The CURL call to the metrics  endpoint doesn't have a timeout. When executing this behind a corporate proxy that blocks this access the filesystem is in a deadlock until curl is killed. By adding a 200ms timeout we give ample time for CURL to finish its request, while also not stopping usage inside local network.
@ras0219
Copy link
Copy Markdown
Contributor

ras0219 commented Mar 2, 2021

Thanks for the PR!

Additionally, we should explicitly release the filesystem lock before spawning the subprocess since there's no need to block another vcpkg invocation in that case. This would enable us to increase the timeout to 3-5 seconds, which should work better on slower connections.

+@strega-nil

Could you make this PR against the new source location: https://github.com/Microsoft/vcpkg-tool ?

@SoftwareApe
Copy link
Copy Markdown
Contributor Author

@ras0219 thank you for the quick response. Yes higher timeout may be better if the lock can be released separately. I didn't take too much time to go through the codebase, just grepped for curl. 🙂

@JackBoosY
Copy link
Copy Markdown
Contributor

@SoftwareApe Please make a same PR to https://github.com/microsoft/vcpkg-tool/pulls.
Thanks.

@SoftwareApe
Copy link
Copy Markdown
Contributor Author

@JackBoosY I created microsoft/vcpkg-tool#22 as requested. I didn't look for how to release the filesystem lock earlier. I also increased the timeout 3s as discussed.

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.

Failed to take the filesystem lock on /home/USER/vcpkg/.vcpkg-root, need to pkill curl to unlock

3 participants