Skip to content

Commit 26319fc

Browse files
committed
Fix webhook sender hanging on destruction when it is still sending.
1 parent 0d095ba commit 26319fc

File tree

2 files changed

+87
-51
lines changed

2 files changed

+87
-51
lines changed

SerialPrograms/Source/Integrations/DiscordWebhook.cpp

Lines changed: 83 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,15 @@ DiscordWebhookSender::DiscordWebhookSender()
4040
{}
4141

4242
DiscordWebhookSender::~DiscordWebhookSender(){
43+
m_stopping.store(true, std::memory_order_release);
4344
{
4445
std::lock_guard<std::mutex> lg(m_lock);
45-
m_stopping = true;
4646
m_cv.notify_all();
4747
}
48+
std::lock_guard<std::mutex> lg(m_send_lock);
49+
if (m_event_loop){
50+
m_event_loop->exit();
51+
}
4852
}
4953

5054
DiscordWebhookSender& DiscordWebhookSender::instance(){
@@ -73,6 +77,7 @@ void DiscordWebhookSender::send(
7377
internal_send(url, *json, attachments);
7478
}
7579
);
80+
// cout << "Scheduling Webhook Message... (queue = " + tostr_u_commas(m_queue.size()) + ")" << endl;
7681
logger.log("Scheduling Webhook Message... (queue = " + tostr_u_commas(m_queue.size()) + ")", COLOR_PURPLE);
7782
}
7883
void DiscordWebhookSender::send(
@@ -96,21 +101,29 @@ void DiscordWebhookSender::send(
96101
internal_send(url, *json, attachments);
97102
}
98103
);
104+
// cout << "Scheduling Webhook Message... (queue = " + tostr_u_commas(m_queue.size()) + ")" << endl;
99105
logger.log("Scheduling Webhook Message... (queue = " + tostr_u_commas(m_queue.size()) + ")", COLOR_PURPLE);
100106
}
101107

102108
void DiscordWebhookSender::cleanup_stuck_requests(){
103-
std::lock_guard<std::mutex> lg(m_lock);
104-
WallClock next = m_queue.next_event();
105-
if (next == WallClock::max()){
106-
return;
109+
{
110+
std::lock_guard<std::mutex> lg(m_lock);
111+
WallClock next = m_queue.next_event();
112+
if (next == WallClock::max()){
113+
return;
114+
}
115+
116+
WallClock now = current_time();
117+
WallClock threshold = now - std::chrono::seconds(60);
118+
if (next >= threshold){
119+
return;
120+
}
107121
}
108122

109-
WallClock now = current_time();
110-
WallClock threshold = now - std::chrono::seconds(60);
111-
if (next < threshold){
112-
m_logger.log("Purging request that appears to be stuck.", COLOR_RED);
113-
emit stop_event_loop();
123+
m_logger.log("Purging request that appears to be stuck.", COLOR_RED);
124+
std::lock_guard<std::mutex> lg(m_send_lock);
125+
if (m_event_loop){
126+
m_event_loop->exit();
114127
}
115128
}
116129
void DiscordWebhookSender::throttle(){
@@ -125,9 +138,9 @@ void DiscordWebhookSender::throttle(){
125138
std::unique_lock<std::mutex> lg(m_lock);
126139
m_cv.wait_for(
127140
lg, duration,
128-
[&]{ return m_stopping || m_sent.empty() || m_sent[0] + duration < now; }
141+
[&]{ return m_stopping.load(std::memory_order_relaxed) || m_sent.empty() || m_sent[0] + duration < now; }
129142
);
130-
if (m_stopping){
143+
if (m_stopping.load(std::memory_order_relaxed)){
131144
return;
132145
}
133146
m_sent.clear();
@@ -159,50 +172,70 @@ void DiscordWebhookSender::internal_send(
159172
const QUrl& url, const JsonValue& json,
160173
const std::vector<DiscordFileAttachment>& files
161174
){
162-
QEventLoop event_loop;
163-
connect(
164-
this, &DiscordWebhookSender::stop_event_loop,
165-
&event_loop, &QEventLoop::quit
166-
);
175+
if (m_stopping.load(std::memory_order_acquire)){
176+
return;
177+
}
167178

168-
QHttpMultiPart multiPart(QHttpMultiPart::FormDataType);
169-
if (!json.is_null()){
170-
QHttpPart json_part;
171-
json_part.setHeader(
172-
QNetworkRequest::ContentDispositionHeader,
173-
QVariant("form-data; name=payload_json")
174-
);
175-
json_part.setBody(QByteArray::fromStdString(json.dump()));
176-
multiPart.append(json_part);
179+
{
180+
std::lock_guard<std::mutex> lg(m_send_lock);
181+
m_event_loop.reset(new QEventLoop);
177182
}
178183

179-
std::vector<QHttpPart> file_parts;
180-
std::deque<QFile> file_readers;
181-
file_parts.reserve(files.size());
182-
size_t c = 0;
183-
for (const auto& file : files){
184-
QFile& reader = file_readers.emplace_back(QString::fromStdString(file.filepath));
185-
if (!reader.open(QIODevice::ReadOnly)){
186-
m_logger.log("File doesn't exist: " + file.filepath, COLOR_RED);
187-
continue;
184+
try{
185+
QHttpMultiPart multiPart(QHttpMultiPart::FormDataType);
186+
if (!json.is_null()){
187+
QHttpPart json_part;
188+
json_part.setHeader(
189+
QNetworkRequest::ContentDispositionHeader,
190+
QVariant("form-data; name=payload_json")
191+
);
192+
json_part.setBody(QByteArray::fromStdString(json.dump()));
193+
multiPart.append(json_part);
188194
}
189-
QHttpPart& part = file_parts.emplace_back();
190-
part.setHeader(
191-
QNetworkRequest::ContentDispositionHeader,
192-
QVariant(QString::fromStdString("application/octet-stream; name=file" + std::to_string(c) + "; filename=" + file.name))
193-
);
194-
part.setBodyDevice(&reader);
195-
multiPart.append(part);
196-
c++;
195+
196+
std::vector<QHttpPart> file_parts;
197+
std::deque<QFile> file_readers;
198+
file_parts.reserve(files.size());
199+
size_t c = 0;
200+
for (const auto& file : files){
201+
QFile& reader = file_readers.emplace_back(QString::fromStdString(file.filepath));
202+
if (!reader.open(QIODevice::ReadOnly)){
203+
m_logger.log("File doesn't exist: " + file.filepath, COLOR_RED);
204+
continue;
205+
}
206+
QHttpPart& part = file_parts.emplace_back();
207+
part.setHeader(
208+
QNetworkRequest::ContentDispositionHeader,
209+
QVariant(QString::fromStdString("application/octet-stream; name=file" + std::to_string(c) + "; filename=" + file.name))
210+
);
211+
part.setBodyDevice(&reader);
212+
multiPart.append(part);
213+
c++;
214+
}
215+
216+
QNetworkRequest request(url);
217+
QNetworkAccessManager manager;
218+
m_event_loop->connect(&manager, SIGNAL(finished(QNetworkReply*)), SLOT(quit()));
219+
// cout << "Sending Webhook Message... (queue = " + tostr_u_commas(m_queue.size()) + ")" << endl;
220+
m_logger.log("Sending Webhook Message... (queue = " + tostr_u_commas(m_queue.size()) + ")", COLOR_BLUE);
221+
std::unique_ptr<QNetworkReply> reply(manager.post(request, &multiPart));
222+
223+
if (!m_stopping.load(std::memory_order_acquire)){
224+
// cout << "internal_send() - exec" << endl;
225+
226+
m_event_loop->exec();
227+
process_reply(reply.get());
228+
229+
// cout << "internal_send() - end" << endl;
230+
}
231+
}catch (...){
232+
std::lock_guard<std::mutex> lg(m_send_lock);
233+
m_event_loop.reset();
234+
throw;
197235
}
198236

199-
QNetworkRequest request(url);
200-
QNetworkAccessManager manager;
201-
event_loop.connect(&manager, SIGNAL(finished(QNetworkReply*)), SLOT(quit()));
202-
m_logger.log("Sending Webhook Message...", COLOR_BLUE);
203-
std::unique_ptr<QNetworkReply> reply(manager.post(request, &multiPart));
204-
event_loop.exec();
205-
process_reply(reply.get());
237+
std::lock_guard<std::mutex> lg(m_send_lock);
238+
m_event_loop.reset();
206239
}
207240

208241

SerialPrograms/Source/Integrations/DiscordWebhook.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,13 @@ class DiscordWebhookSender : public QObject{
7575

7676
private:
7777
TaggedLogger m_logger;
78-
bool m_stopping;
78+
std::atomic<bool> m_stopping;
7979
std::mutex m_lock;
8080
std::condition_variable m_cv;
8181

82+
std::mutex m_send_lock;
83+
std::unique_ptr<QEventLoop> m_event_loop;
84+
8285
std::deque<WallClock> m_sent;
8386
AsyncDispatcher m_dispatcher;
8487
ScheduledTaskRunner m_queue;

0 commit comments

Comments
 (0)