Fix for heap-corruption in Settings::saveAll due to implicitly shared QStrings

Using gflags on windows and enabling full page heap verification, I was able to detect this as a source of
memory corruption.

https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/enable-page-heap

Running interface with pageheap before and after, this change leads me to believe that this is a source of
some of our heap-corruption crashes on backtrace.

Specifically these 19 crashes in the last month, and possibly more.

https://highfidelity.sp.backtrace.io/dashboard/highfidelity/project/Interface/query-builder?qb=%7B%22mode%22%3A%22aggregate%22%2C%22sorting%22%3A%7B%22aggFactor%22%3A%22count%22%2C%22aggSortOrder%22%3A%22descending%22%2C%22selectFactor%22%3A%22timestamp%22%2C%22selectSortOrder%22%3A%22descending%22%2C%22sortedColumnNames%22%3A%5B%5D%7D%2C%22dateTimePicker%22%3A%7B%22granularity%22%3A%221M%22%2C%22start%22%3A%222018-05-27T07%3A00%3A00.000Z%22%2C%22end%22%3A%222018-06-27T23%3A05%3A29.399Z%22%7D%7D&qn=&query=%7B%22filter%22%3A%5B%7B%22callstack%22%3A%5B%5B%22contains%22%2C%22saveAll%22%5D%5D%2C%22timestamp%22%3A%5B%5B%22at-least%22%2C1527404400%5D%2C%5B%22at-most%22%2C1530140729%5D%5D%7D%5D%2C%22fold%22%3A%7B%22timestamp%22%3A%5B%5B%22range%22%5D%2C%5B%22bin%22%2C32%2C1527404400%2C1530140729%5D%5D%7D%2C%22group%22%3A%5B%22classifiers%22%5D%2C%22select%22%3A%5B%22timestamp%22%2C%22fingerprint%22%2C%22_deleted%22%2C%22callstack%22%2C%22object%22%5D%7D
This commit is contained in:
Anthony J. Thibault 2018-06-27 16:06:23 -07:00
parent 7fb2e535ff
commit 70a3c9ce3c

View file

@ -35,5 +35,8 @@ QVariant SettingsScriptingInterface::getValue(const QString& setting, const QVar
}
void SettingsScriptingInterface::setValue(const QString& setting, const QVariant& value) {
Setting::Handle<QVariant>(setting).set(value);
// Make a deep-copy of the string.
// Dangling pointers can occur with QStrings that are implicitly shared from a QScriptEngine.
QString deepCopy = QString::fromUtf16(setting.utf16());
Setting::Handle<QVariant>(deepCopy).set(value);
}