From ee0b5b5e266d1cdd9c4ed144ffc4f4f965db6fdf Mon Sep 17 00:00:00 2001 From: Jonas Date: Mon, 16 Sep 2024 10:50:21 -0400 Subject: [PATCH] ref(profiling) use cleanup instead of destructor (#13661) I think what is happening is that by the time our close handler is called, the handler had already been deleted, which causes a use after free error. I also updated the code to check for is_closing which is what that underlying libuv method actually asserts on. Fixes https://github.com/getsentry/sentry-javascript/pull/13661 --- .../profiling-node/bindings/cpu_profiler.cc | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/packages/profiling-node/bindings/cpu_profiler.cc b/packages/profiling-node/bindings/cpu_profiler.cc index 00996db9e8c9..ad3ca8079e00 100644 --- a/packages/profiling-node/bindings/cpu_profiler.cc +++ b/packages/profiling-node/bindings/cpu_profiler.cc @@ -73,7 +73,7 @@ enum class ProfileStatus { class MeasurementsTicker { private: - uv_timer_t timer; + uv_timer_t *timer; uint64_t period_ms; std::unordered_map> @@ -87,9 +87,10 @@ class MeasurementsTicker { public: MeasurementsTicker(uv_loop_t *loop) : period_ms(100), isolate(v8::Isolate::GetCurrent()) { - uv_timer_init(loop, &timer); - uv_handle_set_data(reinterpret_cast(&timer), this); - uv_unref(reinterpret_cast(&timer)); + timer = new uv_timer_t; + uv_timer_init(loop, timer); + uv_handle_set_data((uv_handle_t *)timer, this); + uv_ref((uv_handle_t *)timer); } static void ticker(uv_timer_t *); @@ -112,13 +113,13 @@ class MeasurementsTicker { size_t listener_count(); ~MeasurementsTicker() { - uv_timer_stop(&timer); + uv_handle_t *handle = (uv_handle_t *)timer; - auto handle = reinterpret_cast(&timer); + uv_timer_stop(timer); + uv_unref(handle); - // Calling uv_close on an inactive handle will cause a segfault. - if (uv_is_active(handle)) { - uv_close(handle, nullptr); + if (!uv_is_closing(handle)) { + uv_close(handle, [](uv_handle_t *handle) { delete handle; }); } } }; @@ -143,8 +144,8 @@ void MeasurementsTicker::add_heap_listener( heap_listeners.emplace(profile_id, cb); if (listener_count() == 1) { - uv_timer_set_repeat(&timer, period_ms); - uv_timer_start(&timer, ticker, 0, period_ms); + uv_timer_set_repeat(timer, period_ms); + uv_timer_start(timer, ticker, 0, period_ms); } } @@ -154,7 +155,7 @@ void MeasurementsTicker::remove_heap_listener( heap_listeners.erase(profile_id); if (listener_count() == 0) { - uv_timer_stop(&timer); + uv_timer_stop(timer); } }; @@ -223,8 +224,8 @@ void MeasurementsTicker::add_cpu_listener( cpu_listeners.emplace(profile_id, cb); if (listener_count() == 1) { - uv_timer_set_repeat(&timer, period_ms); - uv_timer_start(&timer, ticker, 0, period_ms); + uv_timer_set_repeat(timer, period_ms); + uv_timer_start(timer, ticker, 0, period_ms); } } @@ -233,7 +234,7 @@ void MeasurementsTicker::remove_cpu_listener( cpu_listeners.erase(profile_id); if (listener_count() == 0) { - uv_timer_stop(&timer); + uv_timer_stop(timer); } };