Skip to content

Commit aec4aed

Browse files
committed
Fix abort() call on appsec helper unload
runner was living too long (until shared library unload) due to a static shared pointer used for RC notifications. Plus, the destructor would have a call to shared_for_this(), which would try to revive the shared pointer being destroyed, which raise an exception due to there being no shared pointer anymore. We would catch this and abort(). Instead, destroy the runner earlier (when its own thread finishes). Reset the static shared pointer just before that.
1 parent f5c5729 commit aec4aed

File tree

3 files changed

+13
-5
lines changed

3 files changed

+13
-5
lines changed

appsec/src/helper/main.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,8 @@ int appsec_helper_main_impl()
125125

126126
runner->run();
127127

128+
runner->unregister_for_rc_notifications();
129+
128130
finished.store(true, std::memory_order_release);
129131
}};
130132
thread_id = thr.native_handle();

appsec/src/helper/runner.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ void runner::register_for_rc_notifications()
123123
std::atomic_load(&RUNNER_FOR_NOTIFICATIONS);
124124
if (!runner) {
125125
// NOLINTNEXTLINE(bugprone-lambda-function-name)
126-
SPDLOG_ERROR("No runner to notify of remote config updates");
126+
SPDLOG_WARN("No runner to notify of remote config updates");
127127
ddog_remote_config_path_free(path);
128128
return;
129129
}
@@ -136,15 +136,19 @@ void runner::register_for_rc_notifications()
136136
});
137137
}
138138

139-
runner::~runner() noexcept
139+
void runner::unregister_for_rc_notifications()
140140
{
141+
SPDLOG_INFO("Unregister runner for RC update callback");
141142
try {
142143
std::shared_ptr<runner> expected = shared_from_this();
143144
std::atomic_compare_exchange_strong(&RUNNER_FOR_NOTIFICATIONS,
144145
&expected, std::shared_ptr<runner>(nullptr));
145146
} catch (...) {
146-
// can only happened if there is no shared_ptr for the runner
147-
// in this case a std::bad_weak_ptr is thrown
147+
// can only happen if there is no shared_ptr for the runner
148+
// in this case a std::bad_weak_ptr is thrown.
149+
// But we only expose runner through a shared pointer, so this would
150+
// require extraordinary actions to destroy the shared pointer but not
151+
// the object.
148152
std::abort();
149153
}
150154
}

appsec/src/helper/runner.hpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,16 @@ class runner : public std::enable_shared_from_this<runner> {
2626
runner &operator=(const runner &) = delete;
2727
runner(runner &&) = delete;
2828
runner &operator=(runner &&) = delete;
29-
~runner() noexcept;
29+
~runner() = default;
3030

3131
static void resolve_symbols();
3232

3333
void run() noexcept(false);
3434

3535
void register_for_rc_notifications();
3636

37+
void unregister_for_rc_notifications();
38+
3739
[[nodiscard]] bool interrupted() const
3840
{
3941
return interrupted_.load(std::memory_order_acquire);

0 commit comments

Comments
 (0)