Add Comments (for Bug 2673 fix)

Bug 2673 was an important multi-threading issue, and so I added
comments about the code.  Also a LogDebug to track cache use.

Bug 2673 was also a crash with no error message reported.
That is something to revisit later and deserved a comment.
It's believed that the underlying issue is fixed now.

The 'ANSWER-MEs' can be answered in 3.0.1.
This commit is contained in:
James Crook 2021-02-23 12:24:41 +00:00
parent 53f7cacdb2
commit cc9d53df04
2 changed files with 25 additions and 1 deletions

View File

@ -254,6 +254,9 @@ sqlite3_stmt *DBConnection::Prepare(enum StatementID id, const char *sql)
std::lock_guard<std::mutex> guard(mStatementMutex);
int rc;
// See bug 2673
// We must not use the same prepared statement from two different threads.
// Therefore, in the cache, use the thread id too.
StatementIndex ndx(id, std::this_thread::get_id());
// Return an existing statement if it's already been prepared
@ -272,9 +275,23 @@ sqlite3_stmt *DBConnection::Prepare(enum StatementID id, const char *sql)
THROW_INCONSISTENCY_EXCEPTION;
}
// And remember it
// There are a small number (10 or so) of different id's corresponding
// to different SQL statements, see enum StatementID
// We have relatively few threads running at any one time,
// e.g. main gui thread, a playback thread, a thread for compacting.
// However the cache might keep growing, as we start/stop audio,
// perhaps, if we chose to use a new thread each time.
// For 3.0.0 I think that's OK. If it's a data leak it's a slow
// enough one. wxLogDebugs seem to show that the audio play thread
// is being reused, not recreated with a new ID, i.e. no leak at all.
// ANSWER-ME Just how serious is the data leak? How best to fix?
// Remember the cached statement.
mStatements.insert({ndx, stmt});
//Thread Id not convertible to int.
//wxLogDebug( "Cached a statement for thread:%i thread:%i ", (int)ndx.first, (int)ndx.second);
wxLogDebug( "Cached a statement for %i", (int)id);
return stmt;
}

View File

@ -571,6 +571,13 @@ size_t SqliteSampleBlock::GetBlob(void *dest,
// Just showing the user a simple message, not the library error too
// which isn't internationalized
// Actually this can lead to 'Could not read from file' error message
// but it can also lead to no error message at all and a flat line,
// depending on where GetBlob is called from.
// The latter can happen when repainting the screen.
// That possibly happens on a very slow machine. Possibly that's the
// right trade off when a machine can't keep up?
// ANSWER-ME: Do we always report an error when we should here?
Conn()->ThrowException( false );
}