Skip to content

Commit

Permalink
core: fix crash with potential race (#730)
Browse files Browse the repository at this point in the history
Fixes an [issue](envoyproxy/envoy-mobile#729) where the library could potentially race and crash when calling `flushStats` when `server_` has not yet been assigned. @junr03 astutely pointed out that this could be caused by the fact that `server_` is not assigned to anything, so the `if (server_)` check does not perform as expected. To fix this, we should assign it to `nullptr` immediately.

I was able to simulate the race by calling `flushStats()` at the top of `Engine::run()` (before `server_` is assigned). With this patch, the crash disappears.

Resolves envoyproxy/envoy-mobile#729.

Signed-off-by: Michael Rebello <me@michaelrebello.com>
Signed-off-by: JP Simard <jp@jpsim.com>
  • Loading branch information
rebello95 authored and jpsim committed Nov 29, 2022
1 parent 11f5cfa commit c8b38af
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion mobile/library/common/engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class Engine {
std::thread main_thread_;
std::unique_ptr<Http::Dispatcher> http_dispatcher_;
std::unique_ptr<MainCommon> main_common_ GUARDED_BY(mutex_);
Server::Instance* server_;
Server::Instance* server_{};
Server::ServerLifecycleNotifier::HandlePtr postinit_callback_handler_;
Event::Dispatcher* event_dispatcher_;
};
Expand Down

0 comments on commit c8b38af

Please sign in to comment.