Switch db_mutex to a std::mutex. Use modern locking with it. Use zmDbDo or dbQueue.push where appropriate. code cleanup.

This commit is contained in:
Isaac Connor 2021-02-25 12:26:26 -05:00
parent fdf515ca10
commit 8aeb4ab758
9 changed files with 135 additions and 195 deletions

View File

@ -23,7 +23,7 @@
#include <cstdlib>
MYSQL dbconn;
RecursiveMutex db_mutex;
std::mutex db_mutex;
zmDbQueue dbQueue;
bool zmDbConnected = false;
@ -108,46 +108,36 @@ bool zmDbConnect() {
void zmDbClose() {
if (zmDbConnected) {
db_mutex.lock();
std::lock_guard<std::mutex> lck(db_mutex);
mysql_close(&dbconn);
// mysql_init() call implicitly mysql_library_init() but
// mysql_close() does not call mysql_library_end()
mysql_library_end();
zmDbConnected = false;
db_mutex.unlock();
}
}
MYSQL_RES * zmDbFetch(const char * query) {
if ( !zmDbConnected ) {
Error("Not connected.");
return nullptr;
}
db_mutex.lock();
// Might have been disconnected while we waited for the lock
if ( !zmDbConnected ) {
db_mutex.unlock();
std::lock_guard<std::mutex> lck(db_mutex);
if (!zmDbConnected) {
Error("Not connected.");
return nullptr;
}
if ( mysql_query(&dbconn, query) ) {
db_mutex.unlock();
if (mysql_query(&dbconn, query)) {
Error("Can't run query: %s", mysql_error(&dbconn));
return nullptr;
}
Debug(4, "Success running query: %s", query);
MYSQL_RES *result = mysql_store_result(&dbconn);
if ( !result ) {
if (!result) {
Error("Can't use query result: %s for query %s", mysql_error(&dbconn), query);
}
db_mutex.unlock();
return result;
} // end MYSQL_RES * zmDbFetch(const char * query);
zmDbRow *zmDbFetchOne(const char *query) {
zmDbRow *row = new zmDbRow();
if ( row->fetch(query) ) {
if (row->fetch(query)) {
return row;
}
delete row;
@ -156,10 +146,10 @@ zmDbRow *zmDbFetchOne(const char *query) {
MYSQL_RES *zmDbRow::fetch(const char *query) {
result_set = zmDbFetch(query);
if ( ! result_set ) return result_set;
if (!result_set) return result_set;
int n_rows = mysql_num_rows(result_set);
if ( n_rows != 1 ) {
if (n_rows != 1) {
Error("Bogus number of lines return from query, %d returned for query %s.", n_rows, query);
mysql_free_result(result_set);
result_set = nullptr;
@ -167,7 +157,7 @@ MYSQL_RES *zmDbRow::fetch(const char *query) {
}
row = mysql_fetch_row(result_set);
if ( !row ) {
if (!row) {
mysql_free_result(result_set);
result_set = nullptr;
Error("Error getting row from query %s. Error is %s", query, mysql_error(&dbconn));
@ -178,38 +168,34 @@ MYSQL_RES *zmDbRow::fetch(const char *query) {
}
int zmDbDo(const char *query) {
db_mutex.lock();
std::lock_guard<std::mutex> lck(db_mutex);
if (!zmDbConnected)
return 0;
int rc;
while ( (rc = mysql_query(&dbconn, query)) and !zm_terminate) {
db_mutex.unlock();
while ((rc = mysql_query(&dbconn, query)) and !zm_terminate) {
Error("Can't run query %s: %s", query, mysql_error(&dbconn));
if ( (mysql_errno(&dbconn) != ER_LOCK_WAIT_TIMEOUT) )
if ( (mysql_errno(&dbconn) != ER_LOCK_WAIT_TIMEOUT) ) {
return rc;
db_mutex.lock();
}
db_mutex.unlock();
}
return 1;
}
int zmDbDoInsert(const char *query) {
db_mutex.lock();
std::lock_guard<std::mutex> lck(db_mutex);
if (!zmDbConnected) return 0;
int rc;
while ( (rc = mysql_query(&dbconn, query)) and !zm_terminate) {
db_mutex.unlock();
Error("Can't run query %s: %s", query, mysql_error(&dbconn));
if ( (mysql_errno(&dbconn) != ER_LOCK_WAIT_TIMEOUT) )
return 0;
db_mutex.lock();
}
int id = mysql_insert_id(&dbconn);
db_mutex.unlock();
return id;
}
zmDbRow::~zmDbRow() {
if ( result_set ) {
if (result_set) {
mysql_free_result(result_set);
result_set = nullptr;
}

View File

@ -62,7 +62,7 @@ class zmDbRow {
};
extern MYSQL dbconn;
extern RecursiveMutex db_mutex;
extern std::mutex db_mutex;
extern zmDbQueue dbQueue;
extern bool zmDbConnected;

View File

@ -251,11 +251,10 @@ Event::~Event() {
frames, alarm_frames,
tot_score, (int)(alarm_frames?(tot_score/alarm_frames):0), max_score,
id);
db_mutex.lock();
{ // scope for lock
std::lock_guard<std::mutex> lck(db_mutex);
while ( mysql_query(&dbconn, sql) && !zm_terminate ) {
db_mutex.unlock();
Error("Can't update event: %s reason: %s", sql, mysql_error(&dbconn));
db_mutex.lock();
}
if ( !mysql_affected_rows(&dbconn) ) {
// Name might have been changed during recording, so just do the update without changing the name.
@ -267,12 +266,10 @@ Event::~Event() {
tot_score, (int)(alarm_frames?(tot_score/alarm_frames):0), max_score,
id);
while ( mysql_query(&dbconn, sql) && !zm_terminate ) {
db_mutex.unlock();
Error("Can't update event: %s reason: %s", sql, mysql_error(&dbconn));
db_mutex.lock();
}
} // end if no changed rows due to Name change during recording
db_mutex.unlock();
}
} // Event::~Event()
void Event::createNotes(std::string &notes) {
@ -489,14 +486,7 @@ void Event::AddFramesInternal(int n_frames, int start_frame, Image **images, str
if ( frameCount ) {
*(frame_insert_values-1) = '\0';
db_mutex.lock();
int rc = mysql_query(&dbconn, frame_insert_sql);
db_mutex.unlock();
if ( rc ) {
Error("Can't insert frames: %s, sql was (%s)", mysql_error(&dbconn), frame_insert_sql);
} else {
Debug(1, "INSERT %d/%d frames sql %s", frameCount, n_frames, frame_insert_sql);
}
zmDbDo(frame_insert_sql);
last_db_frame = frames;
} else {
Debug(1, "No valid pre-capture frames to add");
@ -548,16 +538,7 @@ void Event::WriteDbFrames() {
delete frame;
}
*(frame_insert_values_ptr-1) = '\0'; // The -1 is for the extra , added for values above
db_mutex.lock();
int rc = mysql_query(&dbconn, frame_insert_sql);
db_mutex.unlock();
if ( rc ) {
Error("Can't insert frames: %s, sql was %s", mysql_error(&dbconn), frame_insert_sql);
return;
} else {
Debug(1, "INSERT FRAMES: sql was %s", frame_insert_sql);
}
zmDbDo(frame_insert_sql);
} // end void Event::WriteDbFrames()
void Event::AddFrame(Image *image, struct timeval timestamp, int score, Image *alarm_image) {

View File

@ -132,7 +132,7 @@ VncCamera::VncCamera(
}
VncCamera::~VncCamera() {
if (capture) {
if (capture and mRfb) {
if (mRfb->frameBuffer)
free(mRfb->frameBuffer);
(*rfbClientCleanup_f)(mRfb);
@ -164,6 +164,7 @@ int VncCamera::PrimeCapture() {
mRfb->GetCredential = GetCredentialsCallback;
mRfb->programName = "Zoneminder VNC Monitor";
if ( mRfb->serverHost ) free(mRfb->serverHost);
mRfb->serverHost = strdup(mHost.c_str());
mRfb->serverPort = atoi(mPort.c_str());
}

View File

@ -21,10 +21,7 @@
#include "zm_db.h"
#include "zm_utils.h"
#include <cerrno>
#include <csignal>
#include <cstdarg>
#include <cstring>
#include <libgen.h>
#include <syslog.h>
#include <sys/time.h>
@ -32,6 +29,11 @@
#ifdef __FreeBSD__
#include <sys/thr.h>
#endif
#include <cerrno>
#include <csignal>
#include <cstdarg>
#include <cstring>
bool Logger::smInitialised = false;
Logger *Logger::smInstance = nullptr;
@ -57,16 +59,15 @@ Logger::Logger() :
mEffectiveLevel(NOLOG),
mDbConnected(false),
mLogPath(staticConfig.PATH_LOGS.c_str()),
//mLogFile( mLogPath+"/"+mId+".log" ),
// mLogFile( mLogPath+"/"+mId+".log" ),
mLogFileFP(nullptr),
mHasTerminal(false),
mFlush(false) {
if ( smInstance ) {
if (smInstance) {
Panic("Attempt to create second instance of Logger class");
}
if ( !smInitialised ) {
if (!smInitialised) {
smCodes[INFO] = "INF";
smCodes[WARNING] = "WAR";
smCodes[ERROR] = "ERR";
@ -81,16 +82,16 @@ Logger::Logger() :
smSyslogPriorities[PANIC] = LOG_ERR;
char code[4] = "";
for ( int i = DEBUG1; i <= DEBUG9; i++ ) {
for (int i = DEBUG1; i <= DEBUG9; i++) {
snprintf(code, sizeof(code), "DB%d", i);
smCodes[i] = code;
smSyslogPriorities[i] = LOG_DEBUG;
}
smInitialised = true;
}
} // end if ! smInitialised
if ( fileno(stderr) && isatty(fileno(stderr)) ) {
if (fileno(stderr) && isatty(fileno(stderr))) {
mHasTerminal = true;
mTerminalLevel = WARNING;
}
@ -101,14 +102,6 @@ Logger::~Logger() {
smCodes.clear();
smSyslogPriorities.clear();
smInitialised = false;
#if 0
for ( StringMap::iterator itr = smCodes.begin(); itr != smCodes.end(); itr ++ ) {
smCodes.erase( itr );
}
for ( IntMap::iterator itr = smSyslogPriorities.begin(); itr != smSyslogPriorities.end(); itr ++ ) {
smSyslogPriorities.erase(itr);
}
#endif
}
void Logger::initialise(const std::string &id, const Options &options) {
@ -355,10 +348,10 @@ Logger::Level Logger::databaseLevel(Logger::Level databaseLevel) {
}
Logger::Level Logger::fileLevel(Logger::Level fileLevel) {
if ( fileLevel > NOOPT ) {
if (fileLevel > NOOPT) {
fileLevel = limit(fileLevel);
// Always close, because we may have changed file names
if ( mFileLevel > NOLOG )
if (mFileLevel > NOLOG)
closeFile();
mFileLevel = fileLevel;
// Don't try to open it here because it will create the log file even if we never write to it.
@ -367,13 +360,13 @@ Logger::Level Logger::fileLevel(Logger::Level fileLevel) {
}
Logger::Level Logger::syslogLevel(Logger::Level syslogLevel) {
if ( syslogLevel > NOOPT ) {
if (syslogLevel > NOOPT) {
syslogLevel = limit(syslogLevel);
if ( mSyslogLevel != syslogLevel ) {
if ( mSyslogLevel > NOLOG )
if (mSyslogLevel != syslogLevel) {
if (mSyslogLevel > NOLOG)
closeSyslog();
mSyslogLevel = syslogLevel;
if ( mSyslogLevel > NOLOG )
if (mSyslogLevel > NOLOG)
openSyslog();
}
}
@ -383,18 +376,18 @@ Logger::Level Logger::syslogLevel(Logger::Level syslogLevel) {
void Logger::logFile(const std::string &logFile) {
bool addLogPid = false;
std::string tempLogFile = logFile;
if ( tempLogFile[tempLogFile.length()-1] == '+' ) {
if (tempLogFile[tempLogFile.length()-1] == '+') {
tempLogFile.resize(tempLogFile.length()-1);
addLogPid = true;
}
if ( addLogPid )
if (addLogPid)
mLogFile = stringtf("%s.%05d", tempLogFile.c_str(), getpid());
else
mLogFile = tempLogFile;
}
void Logger::openFile() {
if ( mLogFile.size() ) {
if (mLogFile.size()) {
if ( (mLogFileFP = fopen(mLogFile.c_str(), "a")) == nullptr ) {
mFileLevel = NOLOG;
Error("fopen() for %s, error = %s", mLogFile.c_str(), strerror(errno));
@ -405,9 +398,9 @@ void Logger::openFile() {
}
void Logger::closeFile() {
if ( mLogFileFP ) {
if (mLogFileFP) {
fflush(mLogFileFP);
if ( fclose(mLogFileFP) < 0 ) {
if (fclose(mLogFileFP) < 0) {
mLogFileFP = nullptr;
Error("fclose(), error = %s", strerror(errno));
}
@ -416,7 +409,6 @@ void Logger::closeFile() {
}
void Logger::closeDatabase() {
}
void Logger::openSyslog() {
@ -428,12 +420,12 @@ void Logger::closeSyslog() {
}
void Logger::logPrint(bool hex, const char * const filepath, const int line, const int level, const char *fstring, ...) {
if ( level > mEffectiveLevel ) {
return;
}
if (level > mEffectiveLevel) return;
if (level < PANIC || level > DEBUG9)
Panic("Invalid logger level %d", level);
log_mutex.lock();
// Can we save some cycles by having these as members and not allocate them on the fly? I think so.
char timeString[64];
char logString[8192];
va_list argPtr;
@ -443,9 +435,6 @@ void Logger::logPrint(bool hex, const char * const filepath, const int line, con
const char *file = base ? base+1 : filepath;
const char *classString = smCodes[level].c_str();
if ( level < PANIC || level > DEBUG9 )
Panic("Invalid logger level %d", level);
gettimeofday(&timeVal, nullptr);
#if 0
@ -474,12 +463,12 @@ void Logger::logPrint(bool hex, const char * const filepath, const int line, con
#else
#ifdef HAVE_SYSCALL
#ifdef __FreeBSD_kernel__
if ( (syscall(SYS_thr_self, &tid)) < 0 ) // Thread/Process id
if ((syscall(SYS_thr_self, &tid)) < 0) // Thread/Process id
# else
// SOLARIS doesn't have SYS_gettid; don't assume
#ifdef SYS_gettid
if ( (tid = syscall(SYS_gettid)) < 0 ) // Thread/Process id
if ((tid = syscall(SYS_gettid)) < 0) // Thread/Process id
#endif // SYS_gettid
#endif
#endif // HAVE_SYSCALL
@ -513,28 +502,29 @@ void Logger::logPrint(bool hex, const char * const filepath, const int line, con
char *syslogEnd = logPtr;
strncpy(logPtr, "]\n", sizeof(logString)-(logPtr-logString));
if ( level <= mTerminalLevel ) {
if (level <= mTerminalLevel) {
puts(logString);
fflush(stdout);
}
if ( level <= mFileLevel ) {
if ( !mLogFileFP ) {
// We do this here so that we only create the file if we ever write to it.
if (level <= mFileLevel) {
if (!mLogFileFP) {
// FIXME unlocking here is a problem. Another thread could sneak in.
log_mutex.unlock();
// We do this here so that we only create the file if we ever write to it.
openFile();
log_mutex.lock();
}
if ( mLogFileFP ) {
if (mLogFileFP) {
fputs(logString, mLogFileFP);
if ( mFlush )
fflush(mLogFileFP);
if (mFlush) fflush(mLogFileFP);
} else {
puts("Logging to file, but failed to open it\n");
}
} // end if level <= mFileLevel
if ( level <= mDatabaseLevel ) {
if (level <= mDatabaseLevel) {
if (zmDbConnected) {
int syslogSize = syslogEnd-syslogStart;
char escapedString[(syslogSize*2)+1];
mysql_real_escape_string(&dbconn, escapedString, syslogStart, syslogSize);
@ -547,27 +537,28 @@ void Logger::logPrint(bool hex, const char * const filepath, const int line, con
timeVal.tv_sec, timeVal.tv_usec, mId.c_str(), staticConfig.SERVER_ID, tid, level, classString, escapedString, file, line
);
dbQueue.push(sql_string);
} else {
puts("Db is closed");
}
} // end if level <= mDatabaseLevel
if ( level <= mSyslogLevel ) {
if (level <= mSyslogLevel) {
*syslogEnd = '\0';
syslog(smSyslogPriorities[level], "%s [%s] [%s]", classString, mId.c_str(), syslogStart);
}
log_mutex.unlock();
if ( level <= FATAL ) {
if (level <= FATAL) {
logTerm();
zmDbClose();
if ( level <= PANIC )
abort();
if (level <= PANIC) abort();
exit(-1);
}
} // end logPrint
void logInit(const char *name, const Logger::Options &options) {
if ( Logger::smInstance ) {
if (Logger::smInstance) {
delete Logger::smInstance;
Logger::smInstance = nullptr;
}
Logger::smInstance = new Logger();
@ -575,7 +566,7 @@ void logInit(const char *name, const Logger::Options &options) {
}
void logTerm() {
if ( Logger::smInstance ) {
if (Logger::smInstance) {
delete Logger::smInstance;
Logger::smInstance = nullptr;
}

View File

@ -153,18 +153,12 @@ public:
void terminate();
const std::string &id(const std::string &id);
const std::string &id() const {
return mId;
}
const std::string &id() const { return mId; }
Level level() const {
return mLevel;
}
Level level() const { return mLevel; }
Level level(Level=NOOPT);
bool debugOn() const {
return mEffectiveLevel >= DEBUG1;
}
bool debugOn() const { return mEffectiveLevel >= DEBUG1; }
Level terminalLevel(Level=NOOPT);
Level databaseLevel(Level=NOOPT);

View File

@ -1692,7 +1692,6 @@ void Monitor::UpdateCaptureFPS() {
last_fps_time = now_double;
last_capture_image_count = image_count;
db_mutex.lock();
static char sql[ZM_SQL_SML_BUFSIZ];
// The reason we update the Status as well is because if mysql restarts, the Monitor_Status table is lost,
// and nothing else will update the status until zmc restarts. Since we are successfully capturing we can
@ -1702,9 +1701,7 @@ void Monitor::UpdateCaptureFPS() {
"VALUES (%d, %.2lf, %u, 'Connected') ON DUPLICATE KEY UPDATE "
"CaptureFPS = %.2lf, CaptureBandwidth=%u, Status='Connected'",
id, new_capture_fps, new_capture_bandwidth, new_capture_fps, new_capture_bandwidth);
int rc = mysql_query(&dbconn, sql);
db_mutex.unlock();
if ( rc ) Error("Can't run query: %s", mysql_error(&dbconn));
dbQueue.push(sql);
} // now != last_fps_time
} // end if report fps
} // void Monitor::UpdateCaptureFPS()
@ -1745,10 +1742,7 @@ void Monitor::UpdateAnalysisFPS() {
"INSERT INTO Monitor_Status (MonitorId,AnalysisFPS) VALUES (%d, %.2lf)"
" ON DUPLICATE KEY UPDATE AnalysisFPS = %.2lf",
id, new_analysis_fps, new_analysis_fps);
db_mutex.lock();
int rc = mysql_query(&dbconn, sql);
db_mutex.unlock();
if ( rc ) Error("Can't run query: %s", mysql_error(&dbconn));
dbQueue.push(sql);
last_analysis_fps_time = now_double;
last_motion_frame_count = motion_frame_count;
} else {
@ -2324,7 +2318,7 @@ void Monitor::ReloadLinkedMonitors(const char *p_linked_monitors) {
for ( int i = 0; i < n_link_ids; i++ ) {
Debug(1, "Checking linked monitor %d", link_ids[i]);
db_mutex.lock();
std::lock_guard<std::mutex> lck(db_mutex);
static char sql[ZM_SQL_SML_BUFSIZ];
snprintf(sql, sizeof(sql),
"SELECT `Id`, `Name` FROM `Monitors`"
@ -2333,14 +2327,12 @@ void Monitor::ReloadLinkedMonitors(const char *p_linked_monitors) {
" AND `Function` != 'Monitor'"
" AND `Enabled`=1",
link_ids[i]);
if ( mysql_query(&dbconn, sql) ) {
db_mutex.unlock();
if (mysql_query(&dbconn, sql)) {
Error("Can't run query: %s", mysql_error(&dbconn));
continue;
}
MYSQL_RES *result = mysql_store_result(&dbconn);
db_mutex.unlock();
if ( !result ) {
Error("Can't use query result: %s", mysql_error(&dbconn));
continue;

View File

@ -134,16 +134,11 @@ Zone::~Zone() {
void Zone::RecordStats(const Event *event) {
static char sql[ZM_SQL_MED_BUFSIZ];
db_mutex.lock();
snprintf(sql, sizeof(sql),
"INSERT INTO Stats SET MonitorId=%d, ZoneId=%d, EventId=%" PRIu64 ", FrameId=%d, PixelDiff=%d, AlarmPixels=%d, FilterPixels=%d, BlobPixels=%d, Blobs=%d, MinBlobSize=%d, MaxBlobSize=%d, MinX=%d, MinY=%d, MaxX=%d, MaxY=%d, Score=%d",
monitor->Id(), id, event->Id(), event->Frames(), pixel_diff, alarm_pixels, alarm_filter_pixels, alarm_blob_pixels, alarm_blobs, min_blob_size, max_blob_size, alarm_box.LoX(), alarm_box.LoY(), alarm_box.HiX(), alarm_box.HiY(), score
);
int rc = mysql_query(&dbconn, sql);
db_mutex.unlock();
if ( rc ) {
Error("Can't insert event stats: %s", mysql_error(&dbconn));
}
zmDbDo(sql);
} // end void Zone::RecordStats( const Event *event )
bool Zone::CheckOverloadCount() {
@ -824,8 +819,10 @@ bool Zone::ParseZoneString(const char *zone_string, int &zone_id, int &colour, P
int Zone::Load(Monitor *monitor, Zone **&zones) {
static char sql[ZM_SQL_MED_BUFSIZ];
MYSQL_RES *result;
db_mutex.lock();
{ // scope for lock
std::lock_guard<std::mutex> lck(db_mutex);
snprintf(sql, sizeof(sql), "SELECT Id,Name,Type+0,Units,Coords,AlarmRGB,CheckMethod+0,"
"MinPixelThreshold,MaxPixelThreshold,MinAlarmPixels,MaxAlarmPixels,"
"FilterX,FilterY,MinFilterPixels,MaxFilterPixels,"
@ -833,14 +830,13 @@ int Zone::Load(Monitor *monitor, Zone **&zones) {
"OverloadFrames,ExtendAlarmFrames"
" FROM Zones WHERE MonitorId = %d ORDER BY Type, Id", monitor->Id());
if ( mysql_query(&dbconn, sql) ) {
db_mutex.unlock();
Error("Can't run query: %s", mysql_error(&dbconn));
return 0;
}
MYSQL_RES *result = mysql_store_result(&dbconn);
db_mutex.unlock();
if ( !result ) {
result = mysql_store_result(&dbconn);
}
if (!result) {
Error("Can't use query result: %s", mysql_error(&dbconn));
return 0;
}
@ -848,7 +844,7 @@ int Zone::Load(Monitor *monitor, Zone **&zones) {
Debug(1, "Got %d zones for monitor %s", n_zones, monitor->Name());
delete[] zones;
zones = new Zone *[n_zones];
for( int i = 0; MYSQL_ROW dbrow = mysql_fetch_row(result); i++ ) {
for(int i = 0; MYSQL_ROW dbrow = mysql_fetch_row(result); i++) {
int col = 0;
int Id = atoi(dbrow[col++]);

View File

@ -266,6 +266,7 @@ int main(int argc, char *argv[]) {
monitor->Id());
zmDbDo(sql);
} // end foreach monitor
if (zm_terminate) break;
#if HAVE_RTSP_SERVER
RTSPServerThread ** rtsp_server_threads = nullptr;
@ -418,9 +419,7 @@ int main(int argc, char *argv[]) {
snprintf(sql, sizeof(sql),
"INSERT INTO Monitor_Status (MonitorId,Status) VALUES (%d, 'Connected') ON DUPLICATE KEY UPDATE Status='NotRunning'",
monitor->Id());
if (mysql_query(&dbconn, sql)) {
Error("Can't run query: %s", mysql_error(&dbconn));
}
zmDbDo(sql);
}
Image::Deinitialise();