1
0
mirror of https://github.com/cookiengineer/audacity synced 2025-06-16 08:09:32 +02:00

Sqlite open bugs (#603)

* Docs say: call sqlite3_close even when sqlite3_open returns error

* Be careful of possible failures of AutoSave...

... In ProjectHistory operations, do the AutoSave first and throw if there is
a failure.

Only then change UndoManager's state, confident that it remains consistent with
what was AutoSaved.

* Throw exceptions if lazy opening of project's database fails...

... because the calls to DB() as in Sqlite3SampleBlock may be in deeply nested
places that can't propagate the error codes; and besides, those functions
had been assuming non-null returns from DB(), which might have crashed before,
but now the assumption is correct when the throw didn't happen.

Note that this exception may also happen during attempted Autosave.  Uses of
Autosave were reviewed and some changes made in the previous commit.
This commit is contained in:
Paul Licameli 2020-07-06 16:42:18 -04:00 committed by GitHub
parent 0c92d1ed74
commit 4206e1ff6d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 37 additions and 9 deletions

View File

@ -235,12 +235,15 @@ ProjectFileIO::~ProjectFileIO()
sqlite3 *ProjectFileIO::DB()
{
if (mDB)
if (!mDB)
{
return mDB;
OpenDB();
if (!mDB)
throw SimpleMessageBoxException{
XO("Failed to open the project's database") };
}
return OpenDB();
return mDB;
}
sqlite3 *ProjectFileIO::OpenDB(FilePath fileName)
@ -266,6 +269,9 @@ sqlite3 *ProjectFileIO::OpenDB(FilePath fileName)
if (rc != SQLITE_OK)
{
SetDBError(XO("Failed to open project file"));
// sqlite3 docs say you should close anyway to avoid leaks
sqlite3_close( mDB );
mDB = nullptr;
return nullptr;
}
@ -647,6 +653,8 @@ bool ProjectFileIO::CopyTo(const FilePath &destpath)
}
else
{
// sqlite3 docs say you should close anyway to avoid leaks
sqlite3_close( destdb );
SetDBError(
XO("Unable to open the destination project file:\n\n%s").Format(destpath)
);
@ -1272,6 +1280,8 @@ bool ProjectFileIO::SaveCopy(const FilePath& fileName)
rc = sqlite3_open(fileName, &db);
if (rc != SQLITE_OK)
{
// sqlite3 docs say you should close anyway to avoid leaks
sqlite3_close( db );
SetDBError(XO("Failed to open backup file"));
return false;
}

View File

@ -109,7 +109,11 @@ private:
static int ExecCallback(void *data, int cols, char **vals, char **names);
int Exec(const char *query, ExecCB callback, wxString *result);
// The opening of the database may be delayed until demanded.
// Returns a non-null pointer to an open database, or throws an exception
// if opening fails.
sqlite3 *DB();
sqlite3 *OpenDB(FilePath fileName = {});
bool CloseDB();
bool DeleteDB();

View File

@ -75,6 +75,15 @@ bool ProjectHistory::RedoAvailable() const
!tracks.HasPendingTracks();
}
namespace {
void AutoSaveOrThrow( ProjectFileIO &projectFileIO )
{
if ( !projectFileIO.AutoSave() )
throw SimpleMessageBoxException{
XO("Automatic database backup failed.") };
}
}
void ProjectHistory::PushState(
const TranslatableString &desc, const TranslatableString &shortDesc)
{
@ -87,6 +96,10 @@ void ProjectHistory::PushState(const TranslatableString &desc,
{
auto &project = mProject;
auto &projectFileIO = ProjectFileIO::Get( project );
if((flags & UndoPush::AUTOSAVE) != UndoPush::MINIMAL)
AutoSaveOrThrow( projectFileIO );
// remaining no-fail operations "commit" the changes of undo manager state
auto &tracks = TrackList::Get( project );
auto &viewInfo = ViewInfo::Get( project );
auto &undoManager = UndoManager::Get( project );
@ -96,9 +109,6 @@ void ProjectHistory::PushState(const TranslatableString &desc,
desc, shortDesc, flags);
mDirty = true;
if((flags & UndoPush::AUTOSAVE) != UndoPush::MINIMAL)
projectFileIO.AutoSave();
}
void ProjectHistory::RollbackState()
@ -112,14 +122,16 @@ void ProjectHistory::ModifyState(bool bWantsAutoSave)
{
auto &project = mProject;
auto &projectFileIO = ProjectFileIO::Get( project );
if (bWantsAutoSave)
AutoSaveOrThrow( projectFileIO );
// remaining no-fail operations "commit" the changes of undo manager state
auto &tracks = TrackList::Get( project );
auto &viewInfo = ViewInfo::Get( project );
auto &undoManager = UndoManager::Get( project );
auto &tags = Tags::Get( project );
undoManager.ModifyState(
&tracks, viewInfo.selectedRegion, tags.shared_from_this());
if (bWantsAutoSave)
projectFileIO.AutoSave();
}
// LL: Is there a memory leak here as "l" and "t" are not deleted???
@ -129,6 +141,9 @@ void ProjectHistory::PopState(const UndoState &state)
{
auto &project = mProject;
auto &projectFileIO = ProjectFileIO::Get( project );
AutoSaveOrThrow( projectFileIO );
// remaining no-fail operations "commit" the changes of undo manager state
auto &dstTracks = TrackList::Get( project );
auto &viewInfo = ViewInfo::Get( project );
@ -146,7 +161,6 @@ void ProjectHistory::PopState(const UndoState &state)
dstTracks.Add(t->Duplicate());
}
projectFileIO.AutoSave();
}
void ProjectHistory::SetStateTo(unsigned int n)