Skip to content

Commit ee0b5b5

Browse files
authored
ref(profiling) use cleanup instead of destructor (getsentry#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 getsentry#13661
1 parent 8d2e189 commit ee0b5b5

File tree

1 file changed

+16
-15
lines changed

1 file changed

+16
-15
lines changed

Diff for: packages/profiling-node/bindings/cpu_profiler.cc

+16-15
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ enum class ProfileStatus {
7373

7474
class MeasurementsTicker {
7575
private:
76-
uv_timer_t timer;
76+
uv_timer_t *timer;
7777
uint64_t period_ms;
7878
std::unordered_map<std::string,
7979
const std::function<bool(uint64_t, v8::HeapStatistics &)>>
@@ -87,9 +87,10 @@ class MeasurementsTicker {
8787
public:
8888
MeasurementsTicker(uv_loop_t *loop)
8989
: period_ms(100), isolate(v8::Isolate::GetCurrent()) {
90-
uv_timer_init(loop, &timer);
91-
uv_handle_set_data(reinterpret_cast<uv_handle_t *>(&timer), this);
92-
uv_unref(reinterpret_cast<uv_handle_t *>(&timer));
90+
timer = new uv_timer_t;
91+
uv_timer_init(loop, timer);
92+
uv_handle_set_data((uv_handle_t *)timer, this);
93+
uv_ref((uv_handle_t *)timer);
9394
}
9495

9596
static void ticker(uv_timer_t *);
@@ -112,13 +113,13 @@ class MeasurementsTicker {
112113
size_t listener_count();
113114

114115
~MeasurementsTicker() {
115-
uv_timer_stop(&timer);
116+
uv_handle_t *handle = (uv_handle_t *)timer;
116117

117-
auto handle = reinterpret_cast<uv_handle_t *>(&timer);
118+
uv_timer_stop(timer);
119+
uv_unref(handle);
118120

119-
// Calling uv_close on an inactive handle will cause a segfault.
120-
if (uv_is_active(handle)) {
121-
uv_close(handle, nullptr);
121+
if (!uv_is_closing(handle)) {
122+
uv_close(handle, [](uv_handle_t *handle) { delete handle; });
122123
}
123124
}
124125
};
@@ -143,8 +144,8 @@ void MeasurementsTicker::add_heap_listener(
143144
heap_listeners.emplace(profile_id, cb);
144145

145146
if (listener_count() == 1) {
146-
uv_timer_set_repeat(&timer, period_ms);
147-
uv_timer_start(&timer, ticker, 0, period_ms);
147+
uv_timer_set_repeat(timer, period_ms);
148+
uv_timer_start(timer, ticker, 0, period_ms);
148149
}
149150
}
150151

@@ -154,7 +155,7 @@ void MeasurementsTicker::remove_heap_listener(
154155
heap_listeners.erase(profile_id);
155156

156157
if (listener_count() == 0) {
157-
uv_timer_stop(&timer);
158+
uv_timer_stop(timer);
158159
}
159160
};
160161

@@ -223,8 +224,8 @@ void MeasurementsTicker::add_cpu_listener(
223224
cpu_listeners.emplace(profile_id, cb);
224225

225226
if (listener_count() == 1) {
226-
uv_timer_set_repeat(&timer, period_ms);
227-
uv_timer_start(&timer, ticker, 0, period_ms);
227+
uv_timer_set_repeat(timer, period_ms);
228+
uv_timer_start(timer, ticker, 0, period_ms);
228229
}
229230
}
230231

@@ -233,7 +234,7 @@ void MeasurementsTicker::remove_cpu_listener(
233234
cpu_listeners.erase(profile_id);
234235

235236
if (listener_count() == 0) {
236-
uv_timer_stop(&timer);
237+
uv_timer_stop(timer);
237238
}
238239
};
239240

0 commit comments

Comments
 (0)