diff --git a/libnymea/integrations/browseractioninfo.cpp b/libnymea/integrations/browseractioninfo.cpp index 6461a678..16a413ff 100644 --- a/libnymea/integrations/browseractioninfo.cpp +++ b/libnymea/integrations/browseractioninfo.cpp @@ -43,11 +43,22 @@ BrowserActionInfo::BrowserActionInfo(Thing *thing, ThingManager *thingManager, c { connect(this, &BrowserActionInfo::finished, this, &BrowserActionInfo::deleteLater, Qt::QueuedConnection); + // TODO 2.0: Create a base class and move this finished() of all Info classes logic handling there + // That will badly break the ABI, so copy/pasting this for now. if (timeout > 0) { QTimer::singleShot(timeout, this, [this] { - emit aborted(); + // It can happen that a plugin calls finish() in a slot which normally would be dispatched before the timeout + // but due to high system load the slot is invoked only after the timeout. This in turn would cause Qt to also queue up + // this timeout slot and by the time the system processes slots, the plugin comes in first and we'd fire an aborted() + // signal in the plugin after it called finish(), potentially badly breaking the plugin as a plugin developer would not + // expect this to happen. So we'll have to verify here that the plugin did not finish() by now before aborting. if (!m_finished) { - finish(Thing::ThingErrorTimeout); + emit aborted(); + // Also it could happen the the plugin calls finish() *in* the aborted() signal, in which case + // we don't want to call finish() here now to not print a warning about duplicate finished calls + if (!m_finished) { + finish(Thing::ThingErrorTimeout); + } } }); } diff --git a/libnymea/integrations/browseresult.cpp b/libnymea/integrations/browseresult.cpp index dbcce180..5148422e 100644 --- a/libnymea/integrations/browseresult.cpp +++ b/libnymea/integrations/browseresult.cpp @@ -44,11 +44,22 @@ BrowseResult::BrowseResult(Thing *thing, ThingManager *thingManager, const QStri { connect(this, &BrowseResult::finished, this, &BrowseResult::deleteLater, Qt::QueuedConnection); + // TODO 2.0: Create a base class and move this finished() of all Info classes logic handling there + // That will badly break the ABI, so copy/pasting this for now. if (timeout > 0) { QTimer::singleShot(timeout, this, [this] { - emit aborted(); + // It can happen that a plugin calls finish() in a slot which normally would be dispatched before the timeout + // but due to high system load the slot is invoked only after the timeout. This in turn would cause Qt to also queue up + // this timeout slot and by the time the system processes slots, the plugin comes in first and we'd fire an aborted() + // signal in the plugin after it called finish(), potentially badly breaking the plugin as a plugin developer would not + // expect this to happen. So we'll have to verify here that the plugin did not finish() by now before aborting. if (!m_finished) { - finish(Thing::ThingErrorTimeout); + emit aborted(); + // Also it could happen the the plugin calls finish() *in* the aborted() signal, in which case + // we don't want to call finish() here now to not print a warning about duplicate finished calls + if (!m_finished) { + finish(Thing::ThingErrorTimeout); + } } }); } diff --git a/libnymea/integrations/browseritemactioninfo.cpp b/libnymea/integrations/browseritemactioninfo.cpp index ead4910e..2d264bff 100644 --- a/libnymea/integrations/browseritemactioninfo.cpp +++ b/libnymea/integrations/browseritemactioninfo.cpp @@ -43,11 +43,22 @@ BrowserItemActionInfo::BrowserItemActionInfo(Thing *thing, ThingManager *thingMa { connect(this, &BrowserItemActionInfo::finished, this, &BrowserItemActionInfo::deleteLater, Qt::QueuedConnection); + // TODO 2.0: Create a base class and move this finished() of all Info classes logic handling there + // That will badly break the ABI, so copy/pasting this for now. if (timeout > 0) { QTimer::singleShot(timeout, this, [this] { - emit aborted(); + // It can happen that a plugin calls finish() in a slot which normally would be dispatched before the timeout + // but due to high system load the slot is invoked only after the timeout. This in turn would cause Qt to also queue up + // this timeout slot and by the time the system processes slots, the plugin comes in first and we'd fire an aborted() + // signal in the plugin after it called finish(), potentially badly breaking the plugin as a plugin developer would not + // expect this to happen. So we'll have to verify here that the plugin did not finish() by now before aborting. if (!m_finished) { - finish(Thing::ThingErrorTimeout); + emit aborted(); + // Also it could happen the the plugin calls finish() *in* the aborted() signal, in which case + // we don't want to call finish() here now to not print a warning about duplicate finished calls + if (!m_finished) { + finish(Thing::ThingErrorTimeout); + } } }); } diff --git a/libnymea/integrations/browseritemresult.cpp b/libnymea/integrations/browseritemresult.cpp index 642f21c7..5a79cc52 100644 --- a/libnymea/integrations/browseritemresult.cpp +++ b/libnymea/integrations/browseritemresult.cpp @@ -44,11 +44,22 @@ BrowserItemResult::BrowserItemResult(Thing *thing, ThingManager *thingManager, c { connect(this, &BrowserItemResult::finished, this, &BrowserItemResult::deleteLater, Qt::QueuedConnection); + // TODO 2.0: Create a base class and move this finished() of all Info classes logic handling there + // That will badly break the ABI, so copy/pasting this for now. if (timeout > 0) { QTimer::singleShot(timeout, this, [this] { - emit aborted(); + // It can happen that a plugin calls finish() in a slot which normally would be dispatched before the timeout + // but due to high system load the slot is invoked only after the timeout. This in turn would cause Qt to also queue up + // this timeout slot and by the time the system processes slots, the plugin comes in first and we'd fire an aborted() + // signal in the plugin after it called finish(), potentially badly breaking the plugin as a plugin developer would not + // expect this to happen. So we'll have to verify here that the plugin did not finish() by now before aborting. if (!m_finished) { - finish(Thing::ThingErrorTimeout); + emit aborted(); + // Also it could happen the the plugin calls finish() *in* the aborted() signal, in which case + // we don't want to call finish() here now to not print a warning about duplicate finished calls + if (!m_finished) { + finish(Thing::ThingErrorTimeout); + } } }); } diff --git a/libnymea/integrations/thingactioninfo.cpp b/libnymea/integrations/thingactioninfo.cpp index 665c822e..d1c0df42 100644 --- a/libnymea/integrations/thingactioninfo.cpp +++ b/libnymea/integrations/thingactioninfo.cpp @@ -44,11 +44,22 @@ ThingActionInfo::ThingActionInfo(Thing *thing, const Action &action, ThingManage { connect(this, &ThingActionInfo::finished, this, &ThingActionInfo::deleteLater, Qt::QueuedConnection); + // TODO 2.0: Create a base class and move this finished() of all Info classes logic handling there + // That will badly break the ABI, so copy/pasting this for now. if (timeout > 0) { QTimer::singleShot(timeout, this, [this] { - emit aborted(); + // It can happen that a plugin calls finish() in a slot which normally would be dispatched before the timeout + // but due to high system load the slot is invoked only after the timeout. This in turn would cause Qt to also queue up + // this timeout slot and by the time the system processes slots, the plugin comes in first and we'd fire an aborted() + // signal in the plugin after it called finish(), potentially badly breaking the plugin as a plugin developer would not + // expect this to happen. So we'll have to verify here that the plugin did not finish() by now before aborting. if (!m_finished) { - finish(Thing::ThingErrorTimeout); + emit aborted(); + // Also it could happen the the plugin calls finish() *in* the aborted() signal, in which case + // we don't want to call finish() here now to not print a warning about duplicate finished calls + if (!m_finished) { + finish(Thing::ThingErrorTimeout); + } } }); } diff --git a/libnymea/integrations/thingdiscoveryinfo.cpp b/libnymea/integrations/thingdiscoveryinfo.cpp index 14aa8e0d..be02c177 100644 --- a/libnymea/integrations/thingdiscoveryinfo.cpp +++ b/libnymea/integrations/thingdiscoveryinfo.cpp @@ -43,11 +43,22 @@ ThingDiscoveryInfo::ThingDiscoveryInfo(const ThingClassId &thingClassId, const P { connect(this, &ThingDiscoveryInfo::finished, this, &ThingDiscoveryInfo::deleteLater, Qt::QueuedConnection); + // TODO 2.0: Create a base class and move this finished() of all Info classes logic handling there + // That will badly break the ABI, so copy/pasting this for now. if (timeout > 0) { QTimer::singleShot(timeout, this, [this] { - emit aborted(); + // It can happen that a plugin calls finish() in a slot which normally would be dispatched before the timeout + // but due to high system load the slot is invoked only after the timeout. This in turn would cause Qt to also queue up + // this timeout slot and by the time the system processes slots, the plugin comes in first and we'd fire an aborted() + // signal in the plugin after it called finish(), potentially badly breaking the plugin as a plugin developer would not + // expect this to happen. So we'll have to verify here that the plugin did not finish() by now before aborting. if (!m_finished) { - finish(Thing::ThingErrorTimeout); + emit aborted(); + // Also it could happen the the plugin calls finish() *in* the aborted() signal, in which case + // we don't want to call finish() here now to not print a warning about duplicate finished calls + if (!m_finished) { + finish(Thing::ThingErrorTimeout); + } } }); } diff --git a/libnymea/integrations/thingpairinginfo.cpp b/libnymea/integrations/thingpairinginfo.cpp index eaa8251a..6e3c4b88 100644 --- a/libnymea/integrations/thingpairinginfo.cpp +++ b/libnymea/integrations/thingpairinginfo.cpp @@ -46,11 +46,22 @@ ThingPairingInfo::ThingPairingInfo(const PairingTransactionId &pairingTransactio { connect(this, &ThingPairingInfo::finished, this, &ThingPairingInfo::deleteLater, Qt::QueuedConnection); + // TODO 2.0: Create a base class and move this finished() of all Info classes logic handling there + // That will badly break the ABI, so copy/pasting this for now. if (timeout > 0) { QTimer::singleShot(timeout, this, [this] { - emit aborted(); + // It can happen that a plugin calls finish() in a slot which normally would be dispatched before the timeout + // but due to high system load the slot is invoked only after the timeout. This in turn would cause Qt to also queue up + // this timeout slot and by the time the system processes slots, the plugin comes in first and we'd fire an aborted() + // signal in the plugin after it called finish(), potentially badly breaking the plugin as a plugin developer would not + // expect this to happen. So we'll have to verify here that the plugin did not finish() by now before aborting. if (!m_finished) { - finish(Thing::ThingErrorTimeout); + emit aborted(); + // Also it could happen the the plugin calls finish() *in* the aborted() signal, in which case + // we don't want to call finish() here now to not print a warning about duplicate finished calls + if (!m_finished) { + finish(Thing::ThingErrorTimeout); + } } }); } diff --git a/libnymea/integrations/thingsetupinfo.cpp b/libnymea/integrations/thingsetupinfo.cpp index 3599fa7e..ec2ef439 100644 --- a/libnymea/integrations/thingsetupinfo.cpp +++ b/libnymea/integrations/thingsetupinfo.cpp @@ -44,11 +44,22 @@ ThingSetupInfo::ThingSetupInfo(Thing *thing, ThingManager *thingManager, quint32 { connect(this, &ThingSetupInfo::finished, this, &ThingSetupInfo::deleteLater, Qt::QueuedConnection); + // TODO 2.0: Create a base class and move this finished() of all Info classes logic handling there + // That will badly break the ABI, so copy/pasting this for now. if (timeout > 0) { QTimer::singleShot(timeout, this, [this] { - emit aborted(); + // It can happen that a plugin calls finish() in a slot which normally would be dispatched before the timeout + // but due to high system load the slot is invoked only after the timeout. This in turn would cause Qt to also queue up + // this timeout slot and by the time the system processes slots, the plugin comes in first and we'd fire an aborted() + // signal in the plugin after it called finish(), potentially badly breaking the plugin as a plugin developer would not + // expect this to happen. So we'll have to verify here that the plugin did not finish() by now before aborting. if (!m_finished) { - finish(Thing::ThingErrorTimeout); + emit aborted(); + // Also it could happen the the plugin calls finish() *in* the aborted() signal, in which case + // we don't want to call finish() here now to not print a warning about duplicate finished calls + if (!m_finished) { + finish(Thing::ThingErrorTimeout); + } } }); }