osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

Fix: Fixed issue with deadlock when same RTP dialog opened twice 

RTP dialogs can stay opened, therefore calls of its functions are
protected by locks. There was issue that same mutex was used during
construction of the dialog and calling functions. It created possible
deadlock.
Change separates lock used for dialog creation and lock for function calls.
When function call lock is locked, new calls are ignored and warning is
printed to STDERR. Showing a dialog with warning looks too intrusive to me.

Fixes #18025
This commit is contained in:
Jirka Novak 2022-04-04 21:21:26 +02:00 committed by j.novak@netsystem.cz
parent 0e269659ec
commit 411b3c1d78
6 changed files with 142 additions and 106 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
ui/qt/rtp_analysis_dialog.cpp
View File

@ -240,11 +240,12 @@ enum {
};
RtpAnalysisDialog *RtpAnalysisDialog::pinstance_{nullptr};
std::mutex RtpAnalysisDialog::mutex_;
std::mutex RtpAnalysisDialog::init_mutex_;
std::mutex RtpAnalysisDialog::run_mutex_;
RtpAnalysisDialog *RtpAnalysisDialog::openRtpAnalysisDialog(QWidget &parent, CaptureFile &cf, QObject *packet_list)
{
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::mutex> lock(init_mutex_);
if (pinstance_ == nullptr)
{
pinstance_ = new RtpAnalysisDialog(parent, cf);
@ -316,13 +317,15 @@ RtpAnalysisDialog::RtpAnalysisDialog(QWidget &parent, CaptureFile &cf) :
RtpAnalysisDialog::~RtpAnalysisDialog()
{
std::lock_guard<std::mutex> lock(mutex_);
delete ui;
for(int i=0; i<tabs_.count(); i++) {
deleteTabInfo(tabs_[i]);
g_free(tabs_[i]);
std::lock_guard<std::mutex> lock(init_mutex_);
if (pinstance_ != nullptr) {
delete ui;
for(int i=0; i<tabs_.count(); i++) {
deleteTabInfo(tabs_[i]);
g_free(tabs_[i]);
}
pinstance_ = nullptr;
}
pinstance_ = nullptr;
}
void RtpAnalysisDialog::deleteTabInfo(tab_info_t *tab_info)
@ -1025,20 +1028,28 @@ void RtpAnalysisDialog::showStreamMenu(QPoint pos)
void RtpAnalysisDialog::replaceRtpStreams(QVector<rtpstream_id_t *> stream_ids)
{
std::lock_guard<std::mutex> lock(mutex_);
// Delete existing tabs (from last to first)
if (tabs_.count() > 0) {
for(int i = static_cast<int>(tabs_.count()); i>0; i--) {
closeTab(i-1);
std::unique_lock<std::mutex> lock(run_mutex_, std::try_to_lock);
if (lock.owns_lock()) {
// Delete existing tabs (from last to first)
if (tabs_.count() > 0) {
for(int i = static_cast<int>(tabs_.count()); i>0; i--) {
closeTab(i-1);
}
}
addRtpStreamsPrivate(stream_ids);
} else {
ws_warning("replaceRtpStreams was called while other thread locked it. Current call is ignored, try it later.");
}
addRtpStreamsPrivate(stream_ids);
}
void RtpAnalysisDialog::addRtpStreams(QVector<rtpstream_id_t *> stream_ids)
{
std::lock_guard<std::mutex> lock(mutex_);
addRtpStreamsPrivate(stream_ids);
std::unique_lock<std::mutex> lock(run_mutex_, std::try_to_lock);
if (lock.owns_lock()) {
addRtpStreamsPrivate(stream_ids);
} else {
ws_warning("addRtpStreams was called while other thread locked it. Current call is ignored, try it later.");
}
}
void RtpAnalysisDialog::addRtpStreamsPrivate(QVector<rtpstream_id_t *> stream_ids)
@ -1089,20 +1100,24 @@ void RtpAnalysisDialog::addRtpStreamsPrivate(QVector<rtpstream_id_t *> stream_id
void RtpAnalysisDialog::removeRtpStreams(QVector<rtpstream_id_t *> stream_ids)
{
std::lock_guard<std::mutex> lock(mutex_);
setUpdatesEnabled(false);
foreach(rtpstream_id_t *id, stream_ids) {
QList<tab_info_t *> tabs = tab_hash_.values(rtpstream_id_to_hash(id));
for (int i = 0; i < tabs.size(); i++) {
tab_info_t *tab = tabs.at(i);
if (rtpstream_id_equal(&tab->stream.id, id, RTPSTREAM_ID_EQUAL_SSRC)) {
closeTab(static_cast<int>(tabs_.indexOf(tab)));
std::unique_lock<std::mutex> lock(run_mutex_, std::try_to_lock);
if (lock.owns_lock()) {
setUpdatesEnabled(false);
foreach(rtpstream_id_t *id, stream_ids) {
QList<tab_info_t *> tabs = tab_hash_.values(rtpstream_id_to_hash(id));
for (int i = 0; i < tabs.size(); i++) {
tab_info_t *tab = tabs.at(i);
if (rtpstream_id_equal(&tab->stream.id, id, RTPSTREAM_ID_EQUAL_SSRC)) {
closeTab(static_cast<int>(tabs_.indexOf(tab)));
}
}
}
}
setUpdatesEnabled(true);
setUpdatesEnabled(true);
updateGraph();
updateGraph();
} else {
ws_warning("removeRtpStreams was called while other thread locked it. Current call is ignored, try it later.");
}
}
tab_info_t *RtpAnalysisDialog::getTabInfoForCurrentTab()

3
ui/qt/rtp_analysis_dialog.h
View File

@ -127,7 +127,8 @@ private slots:
private:
static RtpAnalysisDialog *pinstance_;
static std::mutex mutex_;
static std::mutex init_mutex_;
static std::mutex run_mutex_;
Ui::RtpAnalysisDialog *ui;
enum StreamDirection { dir_all_, dir_one_ };

127
ui/qt/rtp_player_dialog.cpp
View File

@ -134,11 +134,12 @@ public:
};
RtpPlayerDialog *RtpPlayerDialog::pinstance_{nullptr};
std::mutex RtpPlayerDialog::mutex_;
std::mutex RtpPlayerDialog::init_mutex_;
std::mutex RtpPlayerDialog::run_mutex_;
RtpPlayerDialog *RtpPlayerDialog::openRtpPlayerDialog(QWidget &parent, CaptureFile &cf, QObject *packet_list, bool capture_running)
{
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::mutex> lock(init_mutex_);
if (pinstance_ == nullptr)
{
pinstance_ = new RtpPlayerDialog(parent, cf, capture_running);
@ -377,16 +378,18 @@ QToolButton *RtpPlayerDialog::addPlayerButton(QDialogButtonBox *button_box, QDia
#ifdef QT_MULTIMEDIA_LIB
RtpPlayerDialog::~RtpPlayerDialog()
{
std::lock_guard<std::mutex> lock(mutex_);
cleanupMarkerStream();
for (int row = 0; row < ui->streamTreeWidget->topLevelItemCount(); row++) {
QTreeWidgetItem *ti = ui->streamTreeWidget->topLevelItem(row);
RtpAudioStream *audio_stream = ti->data(stream_data_col_, Qt::UserRole).value<RtpAudioStream*>();
if (audio_stream)
delete audio_stream;
std::lock_guard<std::mutex> lock(init_mutex_);
if (pinstance_ != nullptr) {
cleanupMarkerStream();
for (int row = 0; row < ui->streamTreeWidget->topLevelItemCount(); row++) {
QTreeWidgetItem *ti = ui->streamTreeWidget->topLevelItem(row);
RtpAudioStream *audio_stream = ti->data(stream_data_col_, Qt::UserRole).value<RtpAudioStream*>();
if (audio_stream)
delete audio_stream;
}
delete ui;
pinstance_ = nullptr;
}
delete ui;
pinstance_ = nullptr;
}
void RtpPlayerDialog::accept()
@ -773,75 +776,87 @@ void RtpPlayerDialog::unlockUI()
void RtpPlayerDialog::replaceRtpStreams(QVector<rtpstream_id_t *> stream_ids)
{
std::lock_guard<std::mutex> lock(mutex_);
lockUI();
std::unique_lock<std::mutex> lock(run_mutex_, std::try_to_lock);
if (lock.owns_lock()) {
lockUI();
// Delete all existing rows
if (last_ti_) {
highlightItem(last_ti_, false);
last_ti_ = NULL;
}
// Delete all existing rows
if (last_ti_) {
highlightItem(last_ti_, false);
last_ti_ = NULL;
}
for (int row = ui->streamTreeWidget->topLevelItemCount() - 1; row >= 0; row--) {
QTreeWidgetItem *ti = ui->streamTreeWidget->topLevelItem(row);
removeRow(ti);
}
for (int row = ui->streamTreeWidget->topLevelItemCount() - 1; row >= 0; row--) {
QTreeWidgetItem *ti = ui->streamTreeWidget->topLevelItem(row);
removeRow(ti);
}
// Add all new streams
for (int i=0; i < stream_ids.size(); i++) {
addSingleRtpStream(stream_ids[i]);
}
setMarkers();
// Add all new streams
for (int i=0; i < stream_ids.size(); i++) {
addSingleRtpStream(stream_ids[i]);
}
setMarkers();
unlockUI();
unlockUI();
#ifdef QT_MULTIMEDIA_LIB
QTimer::singleShot(0, this, SLOT(retapPackets()));
QTimer::singleShot(0, this, SLOT(retapPackets()));
#endif
} else {
ws_warning("replaceRtpStreams was called while other thread locked it. Current call is ignored, try it later.");
}
}
void RtpPlayerDialog::addRtpStreams(QVector<rtpstream_id_t *> stream_ids)
{
std::lock_guard<std::mutex> lock(mutex_);
lockUI();
std::unique_lock<std::mutex> lock(run_mutex_, std::try_to_lock);
if (lock.owns_lock()) {
lockUI();
int tli_count = ui->streamTreeWidget->topLevelItemCount();
int tli_count = ui->streamTreeWidget->topLevelItemCount();
// Add new streams
for (int i=0; i < stream_ids.size(); i++) {
addSingleRtpStream(stream_ids[i]);
}
// Add new streams
for (int i=0; i < stream_ids.size(); i++) {
addSingleRtpStream(stream_ids[i]);
}
if (tli_count == 0) {
setMarkers();
}
if (tli_count == 0) {
setMarkers();
}
unlockUI();
unlockUI();
#ifdef QT_MULTIMEDIA_LIB
QTimer::singleShot(0, this, SLOT(retapPackets()));
QTimer::singleShot(0, this, SLOT(retapPackets()));
#endif
} else {
ws_warning("addRtpStreams was called while other thread locked it. Current call is ignored, try it later.");
}
}
void RtpPlayerDialog::removeRtpStreams(QVector<rtpstream_id_t *> stream_ids)
{
std::lock_guard<std::mutex> lock(mutex_);
lockUI();
int tli_count = ui->streamTreeWidget->topLevelItemCount();
std::unique_lock<std::mutex> lock(run_mutex_, std::try_to_lock);
if (lock.owns_lock()) {
lockUI();
int tli_count = ui->streamTreeWidget->topLevelItemCount();
for (int i=0; i < stream_ids.size(); i++) {
for (int row = 0; row < tli_count; row++) {
QTreeWidgetItem *ti = ui->streamTreeWidget->topLevelItem(row);
RtpAudioStream *row_stream = ti->data(stream_data_col_, Qt::UserRole).value<RtpAudioStream*>();
if (row_stream->isMatch(stream_ids[i])) {
removeRow(ti);
tli_count--;
break;
for (int i=0; i < stream_ids.size(); i++) {
for (int row = 0; row < tli_count; row++) {
QTreeWidgetItem *ti = ui->streamTreeWidget->topLevelItem(row);
RtpAudioStream *row_stream = ti->data(stream_data_col_, Qt::UserRole).value<RtpAudioStream*>();
if (row_stream->isMatch(stream_ids[i])) {
removeRow(ti);
tli_count--;
break;
}
}
}
}
updateGraphs();
updateGraphs();
updateWidgets();
unlockUI();
updateWidgets();
unlockUI();
} else {
ws_warning("removeRtpStreams was called while other thread locked it. Current call is ignored, try it later.");
}
}
void RtpPlayerDialog::setMarkers()

3
ui/qt/rtp_player_dialog.h
View File

@ -202,7 +202,8 @@ private slots:
#endif
private:
static RtpPlayerDialog *pinstance_;
static std::mutex mutex_;
static std::mutex init_mutex_;
static std::mutex run_mutex_;
#ifdef QT_MULTIMEDIA_LIB
Ui::RtpPlayerDialog *ui;

44
ui/qt/voip_calls_dialog.cpp
View File

@ -44,11 +44,11 @@ enum { voip_calls_type_ = 1000 };
VoipCallsDialog *VoipCallsDialog::pinstance_voip_{nullptr};
VoipCallsDialog *VoipCallsDialog::pinstance_sip_{nullptr};
std::mutex VoipCallsDialog::mutex_;
std::mutex VoipCallsDialog::init_mutex_;
VoipCallsDialog *VoipCallsDialog::openVoipCallsDialogVoip(QWidget &parent, CaptureFile &cf, QObject *packet_list)
{
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::mutex> lock(init_mutex_);
if (pinstance_voip_ == nullptr)
{
pinstance_voip_ = new VoipCallsDialog(parent, cf, false);
@ -60,7 +60,7 @@ VoipCallsDialog *VoipCallsDialog::openVoipCallsDialogVoip(QWidget &parent, Captu
VoipCallsDialog *VoipCallsDialog::openVoipCallsDialogSip(QWidget &parent, CaptureFile &cf, QObject *packet_list)
{
std::lock_guard<std::mutex> lock(mutex_);
std::lock_guard<std::mutex> lock(init_mutex_);
if (pinstance_sip_ == nullptr)
{
pinstance_sip_ = new VoipCallsDialog(parent, cf, true);
@ -195,24 +195,28 @@ bool VoipCallsDialog::eventFilter(QObject *, QEvent *event)
VoipCallsDialog::~VoipCallsDialog()
{
std::lock_guard<std::mutex> lock(mutex_);
delete ui;
std::lock_guard<std::mutex> lock(init_mutex_);
if ((all_flows_ && (pinstance_sip_ != nullptr))
|| (!all_flows_ && (pinstance_voip_ != nullptr))
) {
delete ui;
voip_calls_reset_all_taps(&tapinfo_);
if (!voip_calls_tap_listeners_removed_) {
voip_calls_remove_all_tap_listeners(&tapinfo_);
voip_calls_tap_listeners_removed_ = true;
}
sequence_info_->unref();
g_queue_free(tapinfo_.callsinfos);
// We don't need to clear shown_callsinfos_ data, it was shared
// with tapinfo_.callsinfos and was cleared
// during voip_calls_reset_all_taps
g_queue_free(shown_callsinfos_);
if (all_flows_) {
pinstance_sip_ = nullptr;
} else {
pinstance_voip_ = nullptr;
voip_calls_reset_all_taps(&tapinfo_);
if (!voip_calls_tap_listeners_removed_) {
voip_calls_remove_all_tap_listeners(&tapinfo_);
voip_calls_tap_listeners_removed_ = true;
}
sequence_info_->unref();
g_queue_free(tapinfo_.callsinfos);
// We don't need to clear shown_callsinfos_ data, it was shared
// with tapinfo_.callsinfos and was cleared
// during voip_calls_reset_all_taps
g_queue_free(shown_callsinfos_);
if (all_flows_) {
pinstance_sip_ = nullptr;
} else {
pinstance_voip_ = nullptr;
}
}
}

2
ui/qt/voip_calls_dialog.h
View File

@ -89,7 +89,7 @@ private:
static VoipCallsDialog *pinstance_voip_;
static VoipCallsDialog *pinstance_sip_;
bool all_flows_;
static std::mutex mutex_;
static std::mutex init_mutex_;
Ui::VoipCallsDialog *ui;
VoipCallsInfoModel *call_infos_model_;