Eliminate possible access violation from worker threads while destroying of Platform

[sergz]

Background

If asynchronous operations like sending of web request or timers are executed in background threads then they can cause access violation while destroying of Platform. E.g. between destroying of Platform::webRequest and destroying of Platform::fileSystem some thread from FileSystem implementation can attempt to access already destroyed Platform::webRequest.

What to change

For present I propose to create one mutex which should be used each time we access an asynchronous interface and before accessing it we should check that the pointer is still not nullptr. In ~Platform we should synchronously (using that mutex) move smart pointers to asynchronous interfaces to local variables and let them destroy after releasing of that mutex. This trick in destructor is required to avoid the following dead-lock: let's say we destroy these implementations while the mutex is being acquired, then some async operation which is scheduling another asynchronous operation will be blocked by that mutex but implementation of async interface should wait for finishing of all background threads in the destructor. Thus we block the mutex and wait until finishing of a background thread which is waiting for the mutex.

[sergz]

Basically, it's required only for present. After finishing with #4692 I would expect a payload of a callback passed to async operation to be executed only after acquiring of a strong reference to JsEngine what is already impossible when JsEngine is being destroyed.

[sergz]

Replying to sergz:

Basically, it's required only for present. After finishing with #4692 I would expect a payload of a callback passed to async operation to be executed only after acquiring of a strong reference to JsEngine what is already impossible when JsEngine is being destroyed.

This comment seems incorrect because it's not JsEngine what is managing worker threads (#5198).

The code has also changed, so the description is accordingly updated.

In order to protect access to interface I propose to replace getters by functions acquiring the mutex and calling a callback if the interface is not null, e.g.

void Platform::WithWebRequest(const WithWebRequest& callback) {
  std::lock_guard<std::recusive_mutex> lock(interfacesMutex);
  if (webRequest && callback)
    callback(*webRequest);
}

and usage:

paltform->WithWebRequest([...](IWebRequest& webRequest){
  webRequest->Get(url, headers, [](...){
    ...
  });
});

[abpbot]

A commit referencing this issue has landed:
​Issue 5118 - Lock the platform interfaces before use