Changeset View
Changeset View
Standalone View
Standalone View
src/kdbusservice.cpp
Show First 20 Lines • Show All 136 Lines • ▼ Show 20 Line(s) | 136 | { | |||
---|---|---|---|---|---|
137 | auto bus = QDBusConnection::sessionBus(); | 137 | auto bus = QDBusConnection::sessionBus(); | ||
138 | bool objectRegistered = false; | 138 | bool objectRegistered = false; | ||
139 | objectRegistered = bus.registerObject(QStringLiteral("/MainApplication"), | 139 | objectRegistered = bus.registerObject(QStringLiteral("/MainApplication"), | ||
140 | QCoreApplication::instance(), | 140 | QCoreApplication::instance(), | ||
141 | QDBusConnection::ExportAllSlots | | 141 | QDBusConnection::ExportAllSlots | | ||
142 | QDBusConnection::ExportScriptableProperties | | 142 | QDBusConnection::ExportScriptableProperties | | ||
143 | QDBusConnection::ExportAdaptors); | 143 | QDBusConnection::ExportAdaptors); | ||
144 | if (!objectRegistered) { | 144 | if (!objectRegistered) { | ||
145 | qWarning() << "Failed to register /MainApplication on DBus"; | 145 | qWarning() << "Failed to register /MainApplication on DBus"; | ||
kossebau: What if the quit fails? No error handling?
I fear for having such logic in the library it… | |||||
146 | return; | 146 | return; | ||
147 | } | 147 | } | ||
148 | 148 | | |||
149 | objectRegistered = bus.registerObject(objectPath, s, QDBusConnection::ExportAdaptors); | 149 | objectRegistered = bus.registerObject(objectPath, s, QDBusConnection::ExportAdaptors); | ||
150 | if (!objectRegistered) { | 150 | if (!objectRegistered) { | ||
151 | qWarning() << "Failed to register" << objectPath << "on DBus"; | 151 | qWarning() << "Failed to register" << objectPath << "on DBus"; | ||
152 | return; | 152 | return; | ||
153 | } | 153 | } | ||
Show All 9 Lines | |||||
163 | } | 163 | } | ||
164 | 164 | | |||
165 | void attemptRegistration() | 165 | void attemptRegistration() | ||
166 | { | 166 | { | ||
167 | Q_ASSERT(!d->registered); | 167 | Q_ASSERT(!d->registered); | ||
168 | 168 | | |||
169 | auto queueOption = QDBusConnectionInterface::DontQueueService; | 169 | auto queueOption = QDBusConnectionInterface::DontQueueService; | ||
170 | 170 | | |||
171 | if (options & KDBusService::Unique) { | 171 | if (options & KDBusService::Unique) { | ||
davidedmundson: This is still racey | |||||
172 | // When a process crashes and gets auto-restarted by KCrash we may | 172 | // When a process crashes and gets auto-restarted by KCrash we may | ||
173 | // be in this code path "too early". There is a bit of a delay | 173 | // be in this code path "too early". There is a bit of a delay | ||
174 | // between the restart and the previous process dropping off of the | 174 | // between the restart and the previous process dropping off of the | ||
175 | // bus and thus releasing its registered names. As a result there | 175 | // bus and thus releasing its registered names. As a result there | ||
176 | // is a good chance that if we wait a bit the name will shortly | 176 | // is a good chance that if we wait a bit the name will shortly | ||
177 | // become registered. | 177 | // become registered. | ||
178 | 178 | | |||
179 | queueOption = QDBusConnectionInterface::QueueService; | 179 | queueOption = QDBusConnectionInterface::QueueService; | ||
180 | 180 | | |||
181 | connect(bus, &QDBusConnectionInterface::serviceRegistered, | 181 | connect(bus, &QDBusConnectionInterface::serviceRegistered, | ||
QDBusConnectionInterface::serviceUnregistered is very different to QDBusServiceWatcher::serviceUnregistered QDBusConnectionInterface::serviceUnregistered will be called when our process loses the service name it had. Our process hasn't even requested a service name yet, so this won't happen. davidedmundson: QDBusConnectionInterface::serviceUnregistered is very different to QDBusServiceWatcher… | |||||
182 | this, [this](const QString &service) { | 182 | this, [this](const QString &service) { | ||
183 | if (service != d->serviceName) { | 183 | if (service != d->serviceName) { | ||
184 | return; | 184 | return; | ||
185 | } | 185 | } | ||
186 | 186 | | |||
187 | d->registered = true; | 187 | d->registered = true; | ||
188 | registrationLoop.quit(); | 188 | registrationLoop.quit(); | ||
189 | }); | 189 | }); | ||
190 | } | 190 | } | ||
191 | 191 | | |||
192 | d->registered = | 192 | d->registered = | ||
193 | (bus->registerService(d->serviceName, queueOption) == QDBusConnectionInterface::ServiceRegistered); | 193 | (bus->registerService(d->serviceName, queueOption) == QDBusConnectionInterface::ServiceRegistered); | ||
I don't like this part very much. It means we're registering the name whilst the former app is potentially still running. I would suggest rebasing on Harald's 5bf091ee07ac44ed1bf1e75a4d07847edb86c5d6 change. It'll allow us to do this all in a race-free way. We can register the name org.kde.plasmashell in a queued manner and it'll be better than the current approach plasma does. davidedmundson: I don't like this part very much. It means we're registering the name whilst the former app is… | |||||
194 | 194 | | |||
195 | if (d->registered) { | 195 | if (d->registered) { | ||
196 | return; | 196 | return; | ||
197 | } | 197 | } | ||
198 | 198 | | |||
199 | if (options & KDBusService::Unique) { | 199 | if (options & KDBusService::Replace) { | ||
200 | auto message = QDBusMessage::createMethodCall(d->serviceName, | ||||
201 | QStringLiteral("/MainApplication"), | ||||
202 | QStringLiteral("org.qtproject.Qt.QCoreApplication"), | ||||
203 | QStringLiteral("quit")); | ||||
204 | QDBusConnection::sessionBus().asyncCall(message); //deliberately block until it's done, so we register the name after the app quits | ||||
205 | waitForRegistration(); | ||||
206 | } else if (options & KDBusService::Unique) { | ||||
200 | // Already running so it's ok! | 207 | // Already running so it's ok! | ||
201 | QVariantMap platform_data; | 208 | QVariantMap platform_data; | ||
202 | platform_data.insert(QStringLiteral("desktop-startup-id"), QString::fromUtf8(qgetenv("DESKTOP_STARTUP_ID"))); | 209 | platform_data.insert(QStringLiteral("desktop-startup-id"), QString::fromUtf8(qgetenv("DESKTOP_STARTUP_ID"))); | ||
203 | if (QCoreApplication::arguments().count() > 1) { | 210 | if (QCoreApplication::arguments().count() > 1) { | ||
204 | OrgKdeKDBusServiceInterface iface(d->serviceName, objectPath, QDBusConnection::sessionBus()); | 211 | OrgKdeKDBusServiceInterface iface(d->serviceName, objectPath, QDBusConnection::sessionBus()); | ||
205 | iface.setTimeout(5 * 60 * 1000); // Application can take time to answer | 212 | iface.setTimeout(5 * 60 * 1000); // Application can take time to answer | ||
206 | QDBusReply<int> reply = iface.CommandLine(QCoreApplication::arguments(), QDir::currentPath(), platform_data); | 213 | QDBusReply<int> reply = iface.CommandLine(QCoreApplication::arguments(), QDir::currentPath(), platform_data); | ||
207 | if (reply.isValid()) { | 214 | if (reply.isValid()) { | ||
▲ Show 20 Lines • Show All 140 Lines • Show Last 20 Lines |
What if the quit fails? No error handling?
I fear for having such logic in the library it needs to be more elaborared IMHO.