[PATCH 0/2] MR1528: winhttp: Improve WinHttpGetProxyForUrl() execution time when wpad is not available
Fixes Marvel Snap being unable to connect to servers (part of it, another part is wmic patches), and maybe improves wait times in many other apps. Typically "http://wpad/..." (which is commonly requested as lpszAutoConfigUrl for WinHttpGetProxyForUrl) is not available and WinHttpGetProxyForUrl() is hanging for a really long time resolving this name in download_script(). On Windows 10 here the name resolution timeout is about 9sec (and I could not find a way to configure it for a longer timeout). On Linux default DNS timeouts are typically longer. Yet, while WinHttpGetProxyForUrl() execution time is not consistent here, when I run it right after reboot or after network interface reconnection it typically takes ~4-7 seconds to complete on the first run. Then, testing it again shortly after yields no noticeable timeout. I suppose Windows caches and manages that globally in some service, so maximum possible 9 seconds name resolution timeout is rarely hit. Since proxy config URL, if available, is supposed to be in the local network, having the timeout of 5 seconds for name resolution looks more than enough to me. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1528
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/session.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/winhttp/session.c b/dlls/winhttp/session.c index fa8ccd7a9de..3d99e9b0ac6 100644 --- a/dlls/winhttp/session.c +++ b/dlls/winhttp/session.c @@ -1922,6 +1922,7 @@ static char *download_script( const WCHAR *url, DWORD *out_size ) hostname[uc.dwHostNameLength] = 0; if (!(ses = WinHttpOpen( NULL, WINHTTP_ACCESS_TYPE_NO_PROXY, NULL, NULL, 0 ))) goto done; + WinHttpSetTimeouts( ses, 5000, 60000, 30000, 30000 ); if (!(con = WinHttpConnect( ses, hostname, uc.nPort, 0 ))) goto done; if (uc.nScheme == INTERNET_SCHEME_HTTPS) flags |= WINHTTP_FLAG_SECURE; if (!(req = WinHttpOpenRequest( con, NULL, uc.lpszUrlPath, NULL, NULL, acceptW, flags ))) goto done; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1528
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/session.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/dlls/winhttp/session.c b/dlls/winhttp/session.c index 3d99e9b0ac6..b8a480671de 100644 --- a/dlls/winhttp/session.c +++ b/dlls/winhttp/session.c @@ -1903,15 +1903,41 @@ static BOOL parse_script_result( const char *result, WINHTTP_PROXY_INFO *info ) static char *download_script( const WCHAR *url, DWORD *out_size ) { + static SRWLOCK cache_lock = SRWLOCK_INIT; + static DWORD cached_script_size; + static DWORD cache_update_time; + static char *cached_script; + static WCHAR *cached_url; + static const WCHAR *acceptW[] = {L"*/*", NULL}; HINTERNET ses, con = NULL, req = NULL; WCHAR *hostname; URL_COMPONENTSW uc; DWORD status, size = sizeof(status), offset, to_read, bytes_read, flags = 0; char *tmp, *buffer = NULL; + BOOL cached = FALSE; *out_size = 0; + AcquireSRWLockExclusive( &cache_lock ); + if (cached_url && !wcscmp( cached_url, url ) && GetTickCount() - cache_update_time < 60000) + { + cached = TRUE; + if (cached_script && (buffer = malloc( cached_script_size ))) + { + memcpy( buffer, cached_script, cached_script_size ); + *out_size = cached_script_size; + } + } + ReleaseSRWLockExclusive( &cache_lock ); + + if (cached) + { + TRACE( "Returning cached result.\n" ); + if (!buffer) SetLastError( ERROR_WINHTTP_UNABLE_TO_DOWNLOAD_SCRIPT ); + return buffer; + } + memset( &uc, 0, sizeof(uc) ); uc.dwStructSize = sizeof(uc); uc.dwHostNameLength = -1; @@ -1957,6 +1983,21 @@ done: WinHttpCloseHandle( con ); WinHttpCloseHandle( ses ); free( hostname ); + + AcquireSRWLockExclusive( &cache_lock ); + free( cached_url ); + free( cached_script ); + cached_url = wcsdup( url ); + cached_script_size = 0; + cached_script = NULL; + if (buffer && (cached_script = malloc( *out_size ))) + { + memcpy( cached_script, buffer, *out_size ); + cached_script_size = *out_size; + } + cache_update_time = GetTickCount(); + ReleaseSRWLockExclusive( &cache_lock ); + if (!buffer) SetLastError( ERROR_WINHTTP_UNABLE_TO_DOWNLOAD_SCRIPT ); return buffer; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1528
Hans Leidekker (@hans) commented about dlls/winhttp/session.c:
char *tmp, *buffer = NULL; + BOOL cached = FALSE;
*out_size = 0;
+ AcquireSRWLockExclusive( &cache_lock ); + if (cached_url && !wcscmp( cached_url, url ) && GetTickCount() - cache_update_time < 60000) + { + cached = TRUE; + if (cached_script && (buffer = malloc( cached_script_size ))) + { + memcpy( buffer, cached_script, cached_script_size ); + *out_size = cached_script_size; + } + } + ReleaseSRWLockExclusive( &cache_lock );
I guess we should use GetTickCount64() just in case someone runs a process for more than 49 days :-) This function is getting large, maybe you could introduce some helpers to cache and retrieve the script? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1528#note_17318
Hans Leidekker (@hans) commented about dlls/winhttp/session.c:
WinHttpCloseHandle( con ); WinHttpCloseHandle( ses ); free( hostname ); + + AcquireSRWLockExclusive( &cache_lock ); + free( cached_url ); + free( cached_script ); + cached_url = wcsdup( url ); + cached_script_size = 0; + cached_script = NULL;
Please check for allocation failure. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1528#note_17319
participants (3)
-
Hans Leidekker (＠hans) -
Paul Gofman
-
Paul Gofman (＠gofman)