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

Possible non-return of proxy lookup functions on Windows #17

Open
JohnRusk opened this issue Dec 13, 2019 · 4 comments
Open

Possible non-return of proxy lookup functions on Windows #17

JohnRusk opened this issue Dec 13, 2019 · 4 comments

Comments

@JohnRusk
Copy link
Contributor

Hi Mattn, we recently worked around an issue that we found. Sometimes, about 1 time in a million, looking up a proxy would lock up and never return, on Windows 10.

We put a workaround into AzCopy here: https://github.com/Azure/azure-storage-azcopy/blob/863222d2e2a2bc038f5c35c4faf825ee145c210e/common/ProxyLookupCache.go#L89 . In that code c.lookupMethod == ieproxy.GetProxyFunc(). The workaround calls go-ieproxy in a separate goroutine, so that it can impose a strict timeout with a select statement.

Currently, that workaround is combined with some caching code. It doesn't seem like the caching code would make sense in go-ieproxy. But maybe the workaround would be of interest to you, on its own. (Although, if used without caching, it would have a little more overhead because it uses the separate goroutine for every call).

I'm not sure of the best way to proceed - e.g. does it make sense to put any of this code into go-ieproxy? So at this stage I'm logging this issue rather than creating a PR.

What do you think?

cc @adreed-msft , @zezha-msft

@mattn
Copy link
Owner

mattn commented Dec 13, 2019

I think that we should add ieproxy.GetProxyFuncContext to be possible to cancel request. And context.WithCancel(context.Backup()) to the argument of lookupMethod.

@JohnRusk
Copy link
Contributor Author

I'm not familiar with context.Backup(). Can you explain a bit more about what you propose?

@mattn
Copy link
Owner

mattn commented Jan 9, 2020

Sorry my typo. context.Background() is right.

@JohnRusk
Copy link
Contributor Author

JohnRusk commented Jan 9, 2020

Just checking I've understood correctly: Would that involve moving/copying the cancellable routine that we currently have in AzCopy into go-ieproxy? That sounds good.

I'm a bit snowed under with work right now, so can't make a PR for it soon myself. I might have time later this month if you'd like me to do it. Or if you want to do it, go for it!

klauspost added a commit to klauspost/minio that referenced this issue Jul 10, 2020
Remove windows specific proxy since it causes lockups as per mattn/go-ieproxy#17

Instead use standard Go HTTP proxy.
harshavardhana pushed a commit to minio/minio that referenced this issue Jul 10, 2020
There is a potential for deadlock on Windows 10
refer mattn/go-ieproxy#17 

remove this dependency for now.
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