Re: [2/2] wininet: Free per-thread error info upon exit from an async procedure.

[Jacek Caban]

Hi Hans.

On 12/21/09 11:14 AM, Hans Leidekker wrote:
> Found by Valgrind.
>
> dlls/wininet/internet.c |    6 +++++-
>   1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/dlls/wininet/internet.c b/dlls/wininet/internet.c
> index d749e53..2b8d767 100644
> --- a/dlls/wininet/internet.c
> +++ b/dlls/wininet/internet.c
> @@ -3127,8 +3127,12 @@ static DWORD CALLBACK INTERNET_WorkerThreadFunc(LPVOID lpvParam)
>       HeapFree(GetProcessHeap(), 0, lpRequest);
>   workRequest.asyncproc(&workRequest);
> -
> WININET_Release( workRequest.hdr );
> -
> - if (g_dwTlsErrIndex != TLS_OUT_OF_INDEXES)
> - {
> -
>    HeapFree(GetProcessHeap(), 0, TlsGetValue(g_dwTlsErrIndex));
> - }

You have to set TLS value to NULL otherwise it will crash on next 
INTERNET_SetLastError in the same thread on another async call. But the 
right way to fix this is to get rid of INTERNET_[GS]etLastError. This is 
useless and has proven to result in code that has many failure paths 
without last error set. I've already removed most of these calls and I'm 
near to getting rid of all of them except ones in ftp.c.

Thanks,
     Jacek

[2/2] wininet: Free per-thread error info upon exit from an async procedure.

[Hans Leidekker]

On Monday 21 December 2009 13:09:05 Jacek Caban wrote:
> You have to set TLS value to NULL otherwise it will crash on next 
> INTERNET_SetLastError in the same thread on another async call. But the

Thanks for catching that.
> right way to fix this is to get rid of INTERNET_[GS]etLastError. This is 
> useless and has proven to result in code that has many failure paths 
> without last error set. I've already removed most of these calls and I'm 
> near to getting rid of all of them except ones in ftp.c.

Well, InternetGetLastResponseInfo depends on the per-thread error info and
it may get called (from a callback) on an async thread, so I'd say we do
need to free the TLS value after the async call is done.

-Hans