Eschew use of QStringLiteral in connman bearer plugin

[xhaakon]

The string data QStringLiteral generates is stored in the read-only
segment of the compiled object file. However, libqconnmanbearer.so
can be dynamically unloaded from memory and when this happens, the
data may still be referenced by QStrings e.g. inside Qt DBus. Operation
with such string will lead to application crash because of invalid
memory access.

For this reason change all instances of QStringLiteral to QLatin1String,
with an exception of temporaries used for string comparison, which can't
be referenced from outside of the library.

This problem was observed in crash-reporter-autouploader.

[amccarthy]

Qt plugins are not expected to be unloaded. When is the unloading happening?

[xhaakon]

When is the unloading happening?

It happens on application shutdown, but (at least sometimes) before global QDBusConnection for system bus is deleted. If it still references string literals from the plugin, application crashes in its destructor.

[amccarthy]

It doesn't happen all the time. Is some Qt DBus object constructed by the plugin not being cleaned up on exit?

Using QStringLiteral has performance benefits, I would not like to have it changed to QLatin1String. Do you have a link to the crash-reporter-autouploader report?

[rburchell]

For my comment, I'd say that this is fragile, too. It's way too easy for someone to accidentally add a QSL in the wrong place and reintroduce this problem. It would be nicer to fix it at the 'source' in my opinion.

[xhaakon]

Do you have a link to the crash-reporter-autouploader report?

I've created one for you - see the on-device stacktrace. The crash happens during a destruction of signalHooks QMultiHash, more precisely the invalid string data is referenced inside SignalHook::argumentMatch QStringList. As can be seen in smaps, libqconnmanbearer.so is already unloaded by that time.

When I looked into the plugin code I haven't found any DBus objects that aren't properly cleaned up. Maybe it's that QDBusConnection internally keeps some useless signal hooks even after the objects that were connected to them are deleted.

[amccarthy]

SignalHook is used to store DBus connection information. This is for connections created with QDBusConnection::connect(). A number of calls to this connect method are made from the connman bearer plugin passing QStringLiteral as arguments. These connections not explicitly disconnected anywhere in the code. They will be disconnected when the destination object/slot is destroyed.

QConnmanEngine::connmanServiceInterfaces is populated with QConnmanServiceInterface objects with no parent. They are only deleted when the network service is removed from Connman. I don't think these are being deleted during application shutdown.

All other DBus interface objects seem to have a QObject parent set. I suggest that QConnmanServiceInterface instances created on the heap be created with a parent.

[xhaakon]

Okay, I think I've found a root cause of this problem; it's a bug in Qt DBus. Please follow to #47. I'm closing this PR since it's no longer relevant.