Only tested on Windows/msvc2017. (Shouldn't break other platforms though.)
- Open an existing kmy/xml file. Made some changes to it. And Save.
Save operation should complete without error.
- Verify that the file is present in its location.
Previously save operation on Windows only succeeds every other time. The issue lies in calling QFile::rename() on a QTemporaryFile:
void XMLStorage::saveToLocalFile(const QString& localFile, IMyMoneyOperationsFormat* pWriter, bool plaintext, const QString& keyList) { ... // Create a temporary file if needed QString writeFile = localFile; QTemporaryFile tmpFile; if (QFile::exists(localFile)) { tmpFile.open(); writeFile = tmpFile.fileName(); tmpFile.close(); } ... if (writeFile != localFile) { // [writeFile is in effect tmpFile in this context] if (!QFile::remove(localFile) || !QFile::rename(writeFile, localFile)) throw MYMONEYEXCEPTION(QString::fromLatin1("Failure while writing to '%1'").arg(localFile)); }
(As explained inline in the patch) QTemporaryFile actually keeps the file handle open even after close(). QFile::rename(src, dst) only succeeds when src is 'renamed' (or 'moved' in this context) to dst. As src (i.e. tmpFile) is kept open by QTemporaryFile, the function fails with an error "Cannot remove source file".
This appears to be a bug specific to Windows as Windows does not allow moving files in use.
A quick search reveals it's been a subject of several bug reports: https://bugreports.qt.io/browse/QTBUG-13388, https://bugreports.qt.io/browse/QTBUG-10856, https://bugreports.qt.io/browse/QTBUG-5978; all of which were resolved saying it's "by design".
The fix I adopt is relatively simple: I turn to QFile::copy() instead (which doesn't remove the source file), and just let the QTemporaryFile (the 'source file') deletes itself when it falls out of scope.
Though simple the fix might seem, the consequence of the bug itself could be grave: as shown in the above snippet, when the error is thrown, the original file is already deleted and never restored—a great risk of data loss.
Pressing Save again succeeds though—since the original file doesn't exist and no QTemporaryFile is used. (So Save succeeds on every second time.)
So this patch, I think, should also be backported to the 5.x branch (though there hasn't a Windows release yet)—a Windows release for kmm5 should, in any event, include this patch to prevent some inadvertent data loss.