ETM-Schurig/powersync-core
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix a potential info->aborted() call after the plugin called info->finish()

Browse Source 
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.
This commit is contained in:
Michael Zanetti 2022-08-10 23:29:53 +02:00
parent 7633cfe9fa
commit 3c7e05501e
8 changed files with 104 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
libnymea/integrations/browseractioninfo.cpp
View File

@ -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);
}
}
});
}

15
libnymea/integrations/browseresult.cpp
View File

@ -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);
}
}
});
}

15
libnymea/integrations/browseritemactioninfo.cpp
View File

@ -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);
}
}
});
}

15
libnymea/integrations/browseritemresult.cpp
View File

@ -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);
}
}
});
}

15
libnymea/integrations/thingactioninfo.cpp
View File

@ -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);
}
}
});
}

15
libnymea/integrations/thingdiscoveryinfo.cpp
View File

@ -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);
}
}
});
}

15
libnymea/integrations/thingpairinginfo.cpp
View File

@ -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);
}
}
});
}

15
libnymea/integrations/thingsetupinfo.cpp
View File

@ -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);
}
}
});
}