Skip to content

Isolate watchdog restart to only the timed-out camera #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adameat merged 1 commit into from
Mar 25, 2026
Merged

Isolate watchdog restart to only the timed-out camera #1

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 24 additions & 28 deletions src/motion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,6 @@ void cls_motapp::ntc()
/* Check for whether any cams are locked */
void cls_motapp::watchdog(uint camindx)
{
int indx;

if (cam_list[camindx]->handler_running == false) {
return;
}
Expand All @@ -367,33 +365,31 @@ void cls_motapp::watchdog(uint camindx)
, _("Camera %d - Watchdog timeout.")
, cam_list[camindx]->cfg->device_id);

/* Shut down all the cameras */
for (indx=0; indx<cam_cnt; indx++) {
cam_list[indx]->event_stop = true;
pthread_mutex_unlock(&mutex_camlst);
pthread_mutex_unlock(&mutex_post);
pthread_mutex_unlock(&dbse->mutex_dbse);
pthread_mutex_unlock(&cam_list[indx]->stream.mutex);

if ((cam_list[indx]->camera_type == CAMERA_TYPE_NETCAM) &&
(cam_list[indx]->netcam != nullptr)) {
pthread_mutex_unlock(&cam_list[indx]->netcam->mutex);
pthread_mutex_unlock(&cam_list[indx]->netcam->mutex_pktarray);
pthread_mutex_unlock(&cam_list[indx]->netcam->mutex_transfer);
cam_list[indx]->netcam->handler_stop = true;
}
if ((cam_list[indx]->camera_type == CAMERA_TYPE_NETCAM) &&
(cam_list[indx]->netcam_high != nullptr)) {
pthread_mutex_unlock(&cam_list[indx]->netcam_high->mutex);
pthread_mutex_unlock(&cam_list[indx]->netcam_high->mutex_pktarray);
pthread_mutex_unlock(&cam_list[indx]->netcam_high->mutex_transfer);
cam_list[indx]->netcam_high->handler_stop = true;
}
/* Shut down only the timed-out camera */
cam_list[camindx]->event_stop = true;
pthread_mutex_unlock(&mutex_camlst);

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pthread_mutex_unlock(&mutex_camlst) has the same cross-thread/unlocked-mutex problem as above (undefined behavior with the default pthread_mutex_init(..., NULL) initialization). Additionally, the only mutex_camlst users appear to be camera add/delete and web request validation, so a hung camera handler thread cannot be holding this mutex; unlocking it here is unlikely to help but could corrupt other concurrent operations. Please remove this unlock from the watchdog path (or redesign the watchdog recovery so it doesn’t rely on unlocking mutexes it didn’t lock).

Suggested change
pthread_mutex_unlock(&mutex_camlst);

Copilot uses AI. Check for mistakes.
pthread_mutex_unlock(&mutex_post);

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pthread_mutex_unlock(&mutex_post) in the watchdog path can unlock a mutex that this thread does not own (or that isn’t locked at all), which is undefined behavior with default pthread mutexes. mutex_post appears to be used only by the webcontrol request handlers, not by camera threads, so unlocking it here won’t help recover a hung camera but can break mutual exclusion for concurrent web requests. Please remove this unlock from watchdog() (or replace with a safe recovery mechanism that doesn’t involve cross-thread unlock).

Suggested change
pthread_mutex_unlock(&mutex_post);

Copilot uses AI. Check for mistakes.
pthread_mutex_unlock(&dbse->mutex_dbse);
pthread_mutex_unlock(&cam_list[camindx]->stream.mutex);

if ((cam_list[camindx]->camera_type == CAMERA_TYPE_NETCAM) &&
(cam_list[camindx]->netcam != nullptr)) {
pthread_mutex_unlock(&cam_list[camindx]->netcam->mutex);
pthread_mutex_unlock(&cam_list[camindx]->netcam->mutex_pktarray);
pthread_mutex_unlock(&cam_list[camindx]->netcam->mutex_transfer);
cam_list[camindx]->netcam->handler_stop = true;
}
if ((cam_list[camindx]->camera_type == CAMERA_TYPE_NETCAM) &&
(cam_list[camindx]->netcam_high != nullptr)) {
pthread_mutex_unlock(&cam_list[camindx]->netcam_high->mutex);
pthread_mutex_unlock(&cam_list[camindx]->netcam_high->mutex_pktarray);
pthread_mutex_unlock(&cam_list[camindx]->netcam_high->mutex_transfer);
Comment on lines +372 to +386

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pthread_mutex_unlock(&dbse->mutex_dbse) is undefined behavior unless the watchdog thread owns that mutex. If the intent is to prevent the app from deadlocking when a camera thread hangs inside DB operations, consider using timeouts/timed locks around DB work or moving DB interaction to a dedicated thread/queue so the camera thread doesn’t hold mutex_dbse while performing potentially blocking I/O, rather than forcibly unlocking from another thread.

Suggested change
pthread_mutex_unlock(&dbse->mutex_dbse);
pthread_mutex_unlock(&cam_list[camindx]->stream.mutex);
if ((cam_list[camindx]->camera_type == CAMERA_TYPE_NETCAM) &&
(cam_list[camindx]->netcam != nullptr)) {
pthread_mutex_unlock(&cam_list[camindx]->netcam->mutex);
pthread_mutex_unlock(&cam_list[camindx]->netcam->mutex_pktarray);
pthread_mutex_unlock(&cam_list[camindx]->netcam->mutex_transfer);
cam_list[camindx]->netcam->handler_stop = true;
}
if ((cam_list[camindx]->camera_type == CAMERA_TYPE_NETCAM) &&
(cam_list[camindx]->netcam_high != nullptr)) {
pthread_mutex_unlock(&cam_list[camindx]->netcam_high->mutex);
pthread_mutex_unlock(&cam_list[camindx]->netcam_high->mutex_pktarray);
pthread_mutex_unlock(&cam_list[camindx]->netcam_high->mutex_transfer);
/* Do not attempt to unlock other threads' mutexes here; that is undefined behavior. */
if ((cam_list[camindx]->camera_type == CAMERA_TYPE_NETCAM) &&
(cam_list[camindx]->netcam != nullptr)) {
cam_list[camindx]->netcam->handler_stop = true;
}
if ((cam_list[camindx]->camera_type == CAMERA_TYPE_NETCAM) &&
(cam_list[camindx]->netcam_high != nullptr)) {

Copilot uses AI. Check for mistakes.
cam_list[camindx]->netcam_high->handler_stop = true;
}

cam_list[indx]->handler_shutdown();
if (motsignal != MOTION_SIGNAL_SIGTERM) {
cam_list[indx]->handler_stop = false; /*Trigger a restart*/
}
cam_list[camindx]->handler_shutdown();
if (motsignal != MOTION_SIGNAL_SIGTERM) {
cam_list[camindx]->handler_stop = false; /*Trigger a restart*/
}

}
Expand Down
Loading