RequestServer: Fix crash during process exit
During process exit, static variables and thread-local variables are destroyed in an unpredictable order. The connections HashMap was static, so when destroyed during static destruction, it would destroy ConnectionFromClient objects whose notifiers would try to unregister from thread data that may have already been destroyed. Fix this by moving the connections HashMap from static storage to stack storage in ladybird_main(). This guarantees it will be destroyed before the function returns, while the event loop and all thread data are still fully alive.
This commit is contained in:
parent
ce44ca57ea
commit
22fedde7ef
3 changed files with 18 additions and 5 deletions
|
|
@ -23,16 +23,21 @@
|
|||
|
||||
namespace RequestServer {
|
||||
|
||||
static HashMap<int, RefPtr<ConnectionFromClient>> s_connections;
|
||||
static HashMap<int, NonnullRefPtr<ConnectionFromClient>>* g_connections;
|
||||
static IDAllocator s_client_ids;
|
||||
|
||||
Optional<HTTP::DiskCache> g_disk_cache;
|
||||
|
||||
void ConnectionFromClient::set_connections(HashMap<int, NonnullRefPtr<ConnectionFromClient>>& connections)
|
||||
{
|
||||
g_connections = &connections;
|
||||
}
|
||||
|
||||
ConnectionFromClient::ConnectionFromClient(NonnullOwnPtr<IPC::Transport> transport)
|
||||
: IPC::ConnectionFromClient<RequestClientEndpoint, RequestServerEndpoint>(*this, move(transport), s_client_ids.allocate())
|
||||
, m_resolver(Resolver::default_resolver())
|
||||
{
|
||||
s_connections.set(client_id(), *this);
|
||||
g_connections->set(client_id(), *this);
|
||||
|
||||
m_alt_svc_cache_path = ByteString::formatted("{}/Ladybird/alt-svc-cache.txt", Core::StandardPaths::cache_directory());
|
||||
|
||||
|
|
@ -78,10 +83,10 @@ void ConnectionFromClient::request_complete(Badge<Request>, Request const& reque
|
|||
void ConnectionFromClient::die()
|
||||
{
|
||||
auto client_id = this->client_id();
|
||||
s_connections.remove(client_id);
|
||||
g_connections->remove(client_id);
|
||||
s_client_ids.deallocate(client_id);
|
||||
|
||||
if (s_connections.is_empty())
|
||||
if (g_connections->is_empty())
|
||||
Core::EventLoop::current().quit(0);
|
||||
}
|
||||
|
||||
|
|
@ -137,7 +142,7 @@ ErrorOr<IPC::File> ConnectionFromClient::create_client_socket()
|
|||
return client_socket.release_error();
|
||||
}
|
||||
|
||||
// Note: A ref is stored in the static s_connections map
|
||||
// Note: A ref is stored in the g_connections map
|
||||
auto client = adopt_ref(*new ConnectionFromClient(make<IPC::Transport>(client_socket.release_value())));
|
||||
|
||||
return IPC::File::adopt_fd(socket_fds[1]);
|
||||
|
|
|
|||
|
|
@ -26,6 +26,8 @@ public:
|
|||
|
||||
virtual void die() override;
|
||||
|
||||
static void set_connections(HashMap<int, NonnullRefPtr<ConnectionFromClient>>&);
|
||||
|
||||
void start_revalidation_request(Badge<Request>, ByteString method, URL::URL, NonnullRefPtr<HTTP::HeaderList> request_headers, ByteBuffer request_body, Core::ProxyData proxy_data);
|
||||
void request_complete(Badge<Request>, Request const&);
|
||||
|
||||
|
|
|
|||
|
|
@ -96,6 +96,12 @@ ErrorOr<int> ladybird_main(Main::Arguments arguments)
|
|||
RequestServer::g_disk_cache = cache.release_value();
|
||||
}
|
||||
|
||||
// Connections are stored on the stack to ensure they are destroyed before
|
||||
// static destruction begins. This prevents crashes from notifiers trying to
|
||||
// unregister from already-destroyed thread data during process exit.
|
||||
HashMap<int, NonnullRefPtr<RequestServer::ConnectionFromClient>> connections;
|
||||
RequestServer::ConnectionFromClient::set_connections(connections);
|
||||
|
||||
auto client = TRY(IPC::take_over_accepted_client_from_system_server<RequestServer::ConnectionFromClient>());
|
||||
|
||||
return event_loop.exec();
|
||||
|
|
|
|||
Loading…
Reference in a new issue