Commit 0b6192b3 authored by Guillaume Roguez's avatar Guillaume Roguez

sip: fix SipIceTransport/SipTransportBroker classes

Rationale:
SipIceTranport was not standard-layout but a reinterpret_cast was called
on one member to retrive the class.
This "work" on Linux/gcc implementation but it's an undefined behavior
and not portable.
This changes the internal destroy transport callback:
now PJSIP is the unique owner of the SipIceTransport instance and
should call the destroy callback to delete the instance.
By the way, we remove shutdown and destroy method as not more used.

Side effect is the re-write of SipTransportBroker on how it handles
instance of SipIceTransport.

Refs #64903

Change-Id: I58848eca55c421de1883e53793fd58b0e1581d59
parent 4602105d
......@@ -1019,7 +1019,8 @@ RingAccount::getContactHeader(pjsip_transport* t)
return contact_;
}
auto ice = reinterpret_cast<SipIceTransport*>(t);
// FIXME: be sure that given transport is from SipIceTransport
auto ice = reinterpret_cast<SipIceTransport::TransportData*>(t)->self;
// The transport type must be specified, in our case START_OTHER refers to stun transport
/*pjsip_transport_type_e transportType = transportType_;
......
......@@ -60,19 +60,21 @@ sockaddr_to_host_port(pj_pool_t* pool,
SipIceTransport::SipIceTransport(pjsip_endpoint* endpt, pj_pool_t& /* pool */,
long /* t_type */,
const std::shared_ptr<IceTransport>& ice,
int comp_id, std::function<int()> destroy_cb)
: base()
, pool_(nullptr, pj_pool_release)
int comp_id)
: pool_(nullptr, pj_pool_release)
, rxPool_(nullptr, pj_pool_release)
, rdata()
, trData_()
, rdata_()
, ice_(ice)
, comp_id_(comp_id)
, destroy_cb_(destroy_cb)
{
trData_.self = this;
if (not ice or not ice->isRunning())
throw std::logic_error("ice transport must exist and negotiation completed");
RING_DBG("Creating SipIceTransport");
RING_DBG("SipIceTransport@%p {tr=%p}", this, &trData_.base);
auto& base = trData_.base;
pool_.reset(pjsip_endpt_create_pool(endpt, "SipIceTransport.pool", POOL_TP_INIT, POOL_TP_INC));
if (not pool_)
......@@ -84,7 +86,7 @@ SipIceTransport::SipIceTransport(pjsip_endpoint* endpt, pj_pool_t& /* pool */,
base.tpmgr = pjsip_endpt_get_tpmgr(endpt);
base.pool = pool;
rdata.tp_info.pool = pool;
rdata_.tp_info.pool = pool;
// FIXME: not destroyed in case of exception
if (pj_atomic_create(pool, 0, &base.ref_cnt) != PJ_SUCCESS)
......@@ -122,16 +124,19 @@ SipIceTransport::SipIceTransport(pjsip_endpoint* endpt, pj_pool_t& /* pool */,
pjsip_tx_data *tdata,
const pj_sockaddr_t *rem_addr, int addr_len,
void *token, pjsip_transport_callback callback) {
auto this_ = reinterpret_cast<SipIceTransport*>(transport);
auto& this_ = reinterpret_cast<TransportData*>(transport)->self;
return this_->send(tdata, rem_addr, addr_len, token, callback);
};
base.do_shutdown = [](pjsip_transport *transport){
auto this_ = reinterpret_cast<SipIceTransport*>(transport);
return this_->shutdown();
base.do_shutdown = [](pjsip_transport *transport) -> pj_status_t {
auto& this_ = reinterpret_cast<TransportData*>(transport)->self;
RING_WARN("SipIceTransport@%p: shutdown", this_);
return PJ_SUCCESS;
};
base.destroy = [](pjsip_transport *transport){
auto this_ = reinterpret_cast<SipIceTransport*>(transport);
return this_->destroy();
base.destroy = [](pjsip_transport *transport) -> pj_status_t {
auto& this_ = reinterpret_cast<TransportData*>(transport)->self;
RING_WARN("SipIceTransport@%p: destroy", this_);
delete this_;
return PJ_SUCCESS;
};
/* Init rdata */
......@@ -143,18 +148,18 @@ SipIceTransport::SipIceTransport(pjsip_endpoint* endpt, pj_pool_t& /* pool */,
throw std::bad_alloc();
auto rx_pool = rxPool_.get();
rdata.tp_info.pool = rx_pool;
rdata.tp_info.transport = &base;
rdata.tp_info.tp_data = this;
rdata.tp_info.op_key.rdata = &rdata;
pj_ioqueue_op_key_init(&rdata.tp_info.op_key.op_key, sizeof(pj_ioqueue_op_key_t));
rdata.pkt_info.src_addr = base.key.rem_addr;
rdata.pkt_info.src_addr_len = sizeof(rdata.pkt_info.src_addr);
rdata_.tp_info.pool = rx_pool;
rdata_.tp_info.transport = &base;
rdata_.tp_info.tp_data = this;
rdata_.tp_info.op_key.rdata = &rdata_;
pj_ioqueue_op_key_init(&rdata_.tp_info.op_key.op_key, sizeof(pj_ioqueue_op_key_t));
rdata_.pkt_info.src_addr = base.key.rem_addr;
rdata_.pkt_info.src_addr_len = sizeof(rdata_.pkt_info.src_addr);
auto rem_addr = &base.key.rem_addr;
pj_sockaddr_print(rem_addr, rdata.pkt_info.src_name, sizeof(rdata.pkt_info.src_name), 0);
rdata.pkt_info.src_port = pj_sockaddr_get_port(rem_addr);
rdata.pkt_info.len = 0;
rdata.pkt_info.zero = 0;
pj_sockaddr_print(rem_addr, rdata_.pkt_info.src_name, sizeof(rdata_.pkt_info.src_name), 0);
rdata_.pkt_info.src_port = pj_sockaddr_get_port(rem_addr);
rdata_.pkt_info.len = 0;
rdata_.pkt_info.zero = 0;
if (pjsip_transport_register(base.tpmgr, &base) != PJ_SUCCESS)
throw std::runtime_error("Can't register PJSIP transport.");
......@@ -166,9 +171,11 @@ SipIceTransport::SipIceTransport(pjsip_endpoint* endpt, pj_pool_t& /* pool */,
SipIceTransport::~SipIceTransport()
{
RING_DBG("~SipIceTransport@%p", this);
Manager::instance().unregisterEventHandler((uintptr_t)this);
pj_lock_destroy(base.lock);
pj_atomic_destroy(base.ref_cnt);
pj_lock_destroy(trData_.base.lock);
pj_atomic_destroy(trData_.base.ref_cnt);
RING_DBG("destroying SipIceTransport@%p", this);
}
void
......@@ -224,40 +231,25 @@ SipIceTransport::send(pjsip_tx_data *tdata, const pj_sockaddr_t *rem_addr,
ssize_t
SipIceTransport::onRecv()
{
rdata.pkt_info.len += ice_->recv(comp_id_,
(uint8_t*)rdata.pkt_info.packet+rdata.pkt_info.len,
sizeof(rdata.pkt_info.packet)-rdata.pkt_info.len);
rdata.pkt_info.zero = 0;
pj_gettimeofday(&rdata.pkt_info.timestamp);
rdata_.pkt_info.len += ice_->recv(comp_id_,
(uint8_t*)rdata_.pkt_info.packet+rdata_.pkt_info.len,
sizeof(rdata_.pkt_info.packet)-rdata_.pkt_info.len);
rdata_.pkt_info.zero = 0;
pj_gettimeofday(&rdata_.pkt_info.timestamp);
auto eaten = pjsip_tpmgr_receive_packet(rdata.tp_info.transport->tpmgr, &rdata);
auto eaten = pjsip_tpmgr_receive_packet(rdata_.tp_info.transport->tpmgr, &rdata_);
/* Move unprocessed data to the front of the buffer */
auto rem = rdata.pkt_info.len - eaten;
if (rem > 0 && rem != rdata.pkt_info.len) {
std::move(rdata.pkt_info.packet + eaten,
rdata.pkt_info.packet + eaten + rem,
rdata.pkt_info.packet);
auto rem = rdata_.pkt_info.len - eaten;
if (rem > 0 && rem != rdata_.pkt_info.len) {
std::move(rdata_.pkt_info.packet + eaten,
rdata_.pkt_info.packet + eaten + rem,
rdata_.pkt_info.packet);
}
rdata.pkt_info.len = rem;
rdata_.pkt_info.len = rem;
/* Reset pool */
pj_pool_reset(rdata.tp_info.pool);
}
pj_status_t
SipIceTransport::shutdown()
{
RING_WARN("SIP transport ICE: shutdown");
}
pj_status_t
SipIceTransport::destroy()
{
if (not is_registered_ or not destroy_cb_)
return PJ_SUCCESS;
RING_WARN("SIP transport ICE: destroy");
return destroy_cb_();
pj_pool_reset(rdata_.tp_info.pool);
}
} // namespace ring
......@@ -38,6 +38,7 @@
#include <functional>
#include <memory>
#include <type_traits> // std::is_standard_layout
namespace ring {
......@@ -45,9 +46,20 @@ class IceTransport;
struct SipIceTransport
{
// This structure SHOULD be standard-layout,
// implies std::is_standard_layout<TransportData>::value
// SHOULD return true!
using TransportData = struct {
pjsip_transport base; // do not move, SHOULD be the fist member
SipIceTransport* self {nullptr};
};
static_assert(std::is_standard_layout<TransportData>::value,
"TranportData requires standard-layout");
SipIceTransport(pjsip_endpoint* endpt, pj_pool_t& pool, long t_type,
const std::shared_ptr<IceTransport>& ice,
int comp_id, std::function<int()> destroy_cb);
int comp_id);
~SipIceTransport();
/**
......@@ -61,28 +73,26 @@ struct SipIceTransport
return ice_;
}
pjsip_transport base;
pjsip_transport* getTransportBase() {
return &trData_.base;
}
private:
std::unique_ptr<pj_pool_t, decltype(pj_pool_release)&> pool_;
std::unique_ptr<pj_pool_t, decltype(pj_pool_release)&> rxPool_;
pjsip_rx_data rdata;
TransportData trData_;
pjsip_rx_data rdata_;
bool is_registered_ {false};
const std::shared_ptr<IceTransport> ice_;
const int comp_id_;
std::function<int()> destroy_cb_ {};
pj_status_t send(pjsip_tx_data *tdata, const pj_sockaddr_t *rem_addr,
int addr_len, void *token,
pjsip_transport_callback callback);
ssize_t onRecv();
pj_status_t shutdown();
pj_status_t destroy();
};
} // namespace ring
......@@ -172,6 +172,7 @@ SipTransportBroker::SipTransportBroker(pjsip_endpoint *endpt,
pjsip_transport_get_default_port_for_type(PJSIP_TRANSPORT_UDP),
&ice_pj_transport_type_);
#endif
RING_DBG("SipTransportBroker@%p", this);
}
SipTransportBroker::~SipTransportBroker()
......@@ -183,14 +184,6 @@ SipTransportBroker::~SipTransportBroker()
udpTransports_.clear();
transports_.clear();
if (not iceTransports_.empty()) {
RING_WARN("Remaining %u registred ICE transports",
iceTransports_.size());
for (const auto& tr : iceTransports_)
RING_WARN("+ %p {t=%p {rc=%u}}",
&tr, &tr.base, pj_atomic_get(tr.base.ref_cnt));
}
RING_DBG("destroying SipTransportBroker@%p", this);
}
......@@ -217,26 +210,18 @@ SipTransportBroker::transportStateChanged(pjsip_transport* tp,
sipTransport = key->second.lock();
bool dead = not sipTransport;
#if PJ_VERSION_NUM > (2 << 24 | 1 << 16)
dead |= state == PJSIP_TP_STATE_DESTROY;
bool destroyed = state == PJSIP_TP_STATE_DESTROY;
#else
dead |= tp->is_destroying;
bool destroyed = tp->is_destroying;
#endif
// maps cleanup
if (dead) {
if (destroyed) {
RING_DBG("unmap pjsip transport@%p {SipTransport@%p}",
tp, sipTransport.get());
transports_.erase(key);
// If ICE
const auto iceKey = std::find_if(
iceTransports_.begin(), iceTransports_.end(),
[tp](const SipIceTransport& sit) {
return &sit.base == tp;
});
if (iceKey != iceTransports_.cend())
iceTransports_.erase(iceKey);
// If UDP
const auto type = tp->key.type;
if (type == PJSIP_TRANSPORT_UDP or type == PJSIP_TRANSPORT_UDP6) {
......@@ -248,9 +233,6 @@ SipTransportBroker::transportStateChanged(pjsip_transport* tp,
if (updKey != udpTransports_.cend())
udpTransports_.erase(updKey);
}
RING_DBG("unmapped pjsip transport@%p {SipTransport@%p}",
tp, sipTransport.get());
}
}
......@@ -261,14 +243,23 @@ SipTransportBroker::transportStateChanged(pjsip_transport* tp,
}
std::shared_ptr<SipTransport>
SipTransportBroker::findTransport(pjsip_transport* t)
SipTransportBroker::addTransport(pjsip_transport* t)
{
if (t) {
std::lock_guard<std::mutex> lock(transportMapMutex_);
auto key = transports_.find(t);
if (key != transports_.end()) {
if (auto sipTr = key->second.lock())
return sipTr;
}
auto sipTr = std::make_shared<SipTransport>(t);
if (key != transports_.end())
return key->second.lock();
key->second = sipTr;
else
transports_.emplace(std::make_pair(t, sipTr));
return sipTr;
}
return nullptr;
......@@ -422,13 +413,13 @@ std::shared_ptr<SipTransport>
SipTransportBroker::getIceTransport(const std::shared_ptr<IceTransport> ice,
unsigned comp_id)
{
std::unique_lock<std::mutex> lock(iceMutex_);
iceTransports_.emplace_front(endpt_, pool_, ice_pj_transport_type_, ice,
comp_id, nullptr);
auto& sip_ice_tr = iceTransports_.front();
auto tr = &sip_ice_tr.base;
auto sip_ice_tr = std::unique_ptr<SipIceTransport>(new SipIceTransport(endpt_, pool_,
ice_pj_transport_type_,
ice, comp_id));
auto tr = sip_ice_tr->getTransportBase();
auto sip_tr = std::make_shared<SipTransport>(tr);
sip_ice_tr.release(); // managed by PJSIP now
{
std::unique_lock<std::mutex> lock(transportMapMutex_);
// we do not check for key existance as we've just created it
......
......@@ -165,7 +165,7 @@ public:
std::shared_ptr<SipTransport> getIceTransport(const std::shared_ptr<IceTransport>, unsigned comp_id);
#endif
std::shared_ptr<SipTransport> findTransport(pjsip_transport*);
std::shared_ptr<SipTransport> addTransport(pjsip_transport*);
/**
* Start gracefull shutdown procedure for all transports
......@@ -202,9 +202,6 @@ private:
*/
#if HAVE_DHT
int ice_pj_transport_type_ {PJSIP_TRANSPORT_START_OTHER};
std::list<SipIceTransport> iceTransports_;
std::mutex iceMutex_ {};
#endif
pj_caching_pool& cp_;
......
......@@ -303,7 +303,8 @@ transaction_request_cb(pjsip_rx_data *rdata)
// RING_DBG("transaction_request_cb viaHostname %s toUsername %s addrToUse %s addrSdp %s peerNumber: %s" ,
// viaHostname.c_str(), toUsername.c_str(), addrToUse.toString().c_str(), addrSdp.toString().c_str(), peerNumber.c_str());
auto transport = link->sipTransportBroker->findTransport(rdata->tp_info.transport);
// Append PJSIP transport to the broker's SipTransport list
auto transport = link->sipTransportBroker->addTransport(rdata->tp_info.transport);
if (!transport) {
transport = account->getTransport();
if (!transport) {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment