#1 2019-10-20 15:01:57

zed
Member
From: Belarus
Registered: 2015-02-26
Posts: 105

CurlIsAvailable: possible bug with multi-thread function call

I'm talking about this function:

function CurlIsAvailable: boolean;
begin
 try
   if curl.Module=0 then
     LibCurlInitialize;
   result := PtrInt(curl.Module)>0;
 except
   result := false;
 end;
end;

Now, consider situation when thread A and B calls it at same time (library is not initialized yet).

1. Thread A compares  curl.Module with zero and call LibCurlInitialize.
2. Thread A enters into critical section inside LibCurlInitialize and load dll into memory (call LoadLibrary), now curl.Module <> 0
3. Thread A interrupted halfway (no any GetProcAddress called yet) and Thread B start its work
4. Thread B compares curl.Module with zero and return True
5. Thread B try access to the any function (for example, curl.easy_init) and fails with AV!

To fix this bug we need to use temporary variable for library handle instead immediately writing to curl.Module.
curl.Module should be written only at the exit from LibCurlInitialize when initialization is fully complete.

Last edited by zed (2019-10-20 15:04:08)

Offline

#2 2019-10-20 18:39:02

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,534
Website

Re: CurlIsAvailable: possible bug with multi-thread function call

Yes, this function is not thread safe. We expect CurlIsAvailable is called once in the main thread and if it fails program either exit or fallback to any other library (winHTTP for example under Windows).
I do not see how any temporary variable helps here, the only way is to wrap all function code into a critical section (SynSockCS), but this is IMHO overhead.

Offline

#3 2019-10-20 19:16:11

zed
Member
From: Belarus
Registered: 2015-02-26
Posts: 105

Re: CurlIsAvailable: possible bug with multi-thread function call

LibCurlInitialize can be refactored with additional variable wich solves the problem. I will create pull request with fix tomorrow.

Offline

#4 2019-10-21 06:43:40

zed
Member
From: Belarus
Registered: 2015-02-26
Posts: 105

Re: CurlIsAvailable: possible bug with multi-thread function call

Offline

Board footer

Powered by FluxBB