jamiaccount: avoid deadlock when sending a SIP message

shutdownSIPConnection must not be called in pjsip callback (can be
called in shutdownSIPConnection)

GitLab: #775
Change-Id: I4d05d38e863c0f2b1f4e72d2fc827d17ca4bad8b
This commit is contained in:
Sébastien Blin
2022-10-14 17:14:59 -04:00
parent 632643de48
commit 59cb825df5
2 changed files with 51 additions and 33 deletions

View File

@ -3223,32 +3223,16 @@ JamiAccount::sendMessage(const std::string& to,
try {
auto res = sendSIPMessage(
conn, to, ctx.release(), token, payloads, [](void* token, pjsip_event* event) {
std::unique_ptr<TextMessageCtx> c {(TextMessageCtx*) token};
std::shared_ptr<TextMessageCtx> c {(TextMessageCtx*) token};
auto code = event->body.tsx_state.tsx->status_code;
auto acc = c->acc.lock();
if (not acc)
return;
if (code == PJSIP_SC_OK) {
std::unique_lock<std::mutex> l(c->confirmation->lock);
c->confirmation->replied = true;
l.unlock();
if (!c->onlyConnected)
acc->messageEngine_.onMessageSent(c->to, c->id, true);
} else {
JAMI_WARN("Timeout when send a message, close current connection");
acc->shutdownSIPConnection(c->channel, c->to, c->deviceId);
// This MUST be done after closing the connection to avoid race condition
// with messageEngine_
if (!c->onlyConnected)
acc->messageEngine_.onMessageSent(c->to, c->id, false);
// In that case, the peer typically changed its connectivity.
// After closing sockets with that peer, we try to re-connect to
// that peer one time.
if (c->retryOnTimeout)
acc->messageEngine_.onPeerOnline(c->to, false);
}
runOnMainThread([c=std::move(c), code]() {
if (c) {
auto acc = c->acc.lock();
if (not acc)
return;
acc->onSIPMessageSent(std::move(c), code);
}
});
});
if (!res) {
if (!onlyConnected)
@ -3401,6 +3385,36 @@ JamiAccount::sendMessage(const std::string& to,
std::chrono::minutes(1));
}
void
JamiAccount::onSIPMessageSent(const std::shared_ptr<TextMessageCtx>& ctx, int code)
{
if (code == PJSIP_SC_OK) {
std::unique_lock<std::mutex> l(ctx->confirmation->lock);
ctx->confirmation->replied = true;
l.unlock();
if (!ctx->onlyConnected)
messageEngine_.onMessageSent(ctx->to, ctx->id, true);
} else {
// Note: This can be called from pjsip's eventloop while
// sipConnsMtx_ is locked. So we should retrigger the shutdown.
auto acc = ctx->acc.lock();
if (not acc)
return;
JAMI_WARN("Timeout when send a message, close current connection");
shutdownSIPConnection(ctx->channel, ctx->to, ctx->deviceId);
// This MUST be done after closing the connection to avoid race condition
// with messageEngine_
if (!ctx->onlyConnected)
messageEngine_.onMessageSent(ctx->to, ctx->id, false);
// In that case, the peer typically changed its connectivity.
// After closing sockets with that peer, we try to re-connect to
// that peer one time.
if (ctx->retryOnTimeout)
messageEngine_.onPeerOnline(ctx->to, false);
}
}
void
JamiAccount::onIsComposing(const std::string& conversationId,
const std::string& peer,
@ -4111,14 +4125,16 @@ JamiAccount::cacheSIPConnection(std::shared_ptr<ChannelSocket>&& socket,
// Convert to SIP transport
auto onShutdown = [w = weak(), peerId, key, socket]() {
auto shared = w.lock();
if (!shared)
return;
shared->shutdownSIPConnection(socket, key.first, key.second);
// The connection can be closed during the SIP initialization, so
// if this happens, the request should be re-sent to ask for a new
// SIP channel to make the call pass through
shared->callConnectionClosed(key.second, false);
runOnMainThread([w=std::move(w), peerId, key, socket] {
auto shared = w.lock();
if (!shared)
return;
shared->shutdownSIPConnection(socket, key.first, key.second);
// The connection can be closed during the SIP initialization, so
// if this happens, the request should be re-sent to ask for a new
// SIP channel to make the call pass through
shared->callConnectionClosed(key.second, false);
});
};
auto sip_tr = link_.sipTransportBroker->getChanneledTransport(shared(),
socket,

View File

@ -90,6 +90,7 @@ struct AccountInfo;
class SipTransport;
class ChanneledOutgoingTransfer;
class SyncModule;
struct TextMessageCtx;
using SipConnectionKey = std::pair<std::string /* accountId */, DeviceId>;
using GitSocketList = std::map<DeviceId, /* device Id */
@ -929,6 +930,7 @@ private:
uint64_t token,
const std::map<std::string, std::string>& data,
pjsip_endpt_send_callback cb);
void onSIPMessageSent(const std::shared_ptr<TextMessageCtx>& ctx, int code);
std::mutex gitServersMtx_ {};
std::map<dht::Value::Id, std::unique_ptr<GitServer>> gitServers_ {};