You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This appears to be a racy (albeit with a narrow window), because finished is made true before is the callback queue is appended to. If the callback queue was previously empty, it would appear that the NetworkHTTPRequest instance is liable for deletion via NetworkHTTPSocketHandler::HTTPReceive and NetworkHTTPRequest::Receive in the gap after finished is set to true, but before callback.OnFailure is called. It would seem that this could result in the call to callback.OnFailure being a use after free because *this has already been destructed.
Probably finished should be assigned after calling callback.OnFailure (i.e. release semantics), and in NetworkHTTPRequest::Receive it should be read once, first (i.e. acquire semantics).
Steps to reproduce
See http_winhttp.cpp
The text was updated successfully, but these errors were encountered:
Version of OpenTTD
master
Expected result
No possible race conditions, etc.
Actual result
In the WinHTTP backend there are various instances of this pattern:
where
this
is a NetworkHTTPRequest.This appears to be a racy (albeit with a narrow window), because finished is made true before is the callback queue is appended to. If the callback queue was previously empty, it would appear that the NetworkHTTPRequest instance is liable for deletion via
NetworkHTTPSocketHandler::HTTPReceive
andNetworkHTTPRequest::Receive
in the gap after finished is set to true, but before callback.OnFailure is called. It would seem that this could result in the call tocallback.OnFailure
being a use after free because*this
has already been destructed.Probably
finished
should be assigned after callingcallback.OnFailure
(i.e. release semantics), and inNetworkHTTPRequest::Receive
it should be read once, first (i.e. acquire semantics).Steps to reproduce
See http_winhttp.cpp
The text was updated successfully, but these errors were encountered: