From ace66cf977fdec1d5d8427a09f098b9ca2cb658e Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Wed, 17 Jan 2018 13:27:44 -0800 Subject: [PATCH 1/5] show port in source column --- web/skins/classic/views/console.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/skins/classic/views/console.php b/web/skins/classic/views/console.php index c8ec29020..c4728fc0b 100644 --- a/web/skins/classic/views/console.php +++ b/web/skins/classic/views/console.php @@ -230,8 +230,8 @@ for( $monitor_i = 0; $monitor_i < count($displayMonitors); $monitor_i += 1 ) { } elseif ( $monitor['Type'] == 'File' || $monitor['Type'] == 'cURL' ) { $source = preg_replace( '/^.*\//', '', $monitor['Path'] ); } elseif ( $monitor['Type'] == 'Ffmpeg' || $monitor['Type'] == 'Libvlc' ) { - $domain = parse_url( $monitor['Path'], PHP_URL_HOST ); - $source = $domain ? $domain : preg_replace( '/^.*\//', '', $monitor['Path'] ); + $url_parts = parse_url( $monitor['Path'] ); + $source = $url_parts['host']. ( $url_parts['port'] ? ':'.$url_parts['port'] : '' ); } if ( $source == '' ) { $source = 'Monitor ' . $monitor['Id']; From 715adb5acb2d07c49aa6bbae706b55c40e68bb16 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 18 Jan 2018 11:38:08 -0500 Subject: [PATCH 2/5] add locking to Event moving and diskspace updating. --- scripts/ZoneMinder/lib/ZoneMinder/Event.pm | 25 ++++++++++++-- scripts/ZoneMinder/lib/ZoneMinder/Object.pm | 38 +++++++++++++++++++++ scripts/zmfilter.pl.in | 4 +++ 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/scripts/ZoneMinder/lib/ZoneMinder/Event.pm b/scripts/ZoneMinder/lib/ZoneMinder/Event.pm index 8f4a276f7..8218c763d 100644 --- a/scripts/ZoneMinder/lib/ZoneMinder/Event.pm +++ b/scripts/ZoneMinder/lib/ZoneMinder/Event.pm @@ -431,6 +431,14 @@ sub DiskSpace { sub MoveTo { my ( $self, $NewStorage ) = @_; + $ZoneMinder::Database::dbh->begin_work(); + $self->lock_and_load(); + # data is reloaded, so need to check that the move hasn't already happened. + if ( $$self{StorageId} == $$NewStorage{Id} ) { + $ZoneMinder::Database::dbh->commit(); + return "Event has already been moved by someone else."; + } + my $OldStorage = $self->Storage(); my ( $OldPath ) = ( $self->Path() =~ /^(.*)$/ ); # De-taint @@ -438,16 +446,21 @@ sub MoveTo { my ( $NewPath ) = ( $NewStorage->Path() =~ /^(.*)$/ ); # De-taint if ( ! $$NewStorage{Id} ) { + $ZoneMinder::Database::dbh->commit(); return "New storage does not have an id. Moving will not happen."; } elsif ( !$NewPath ) { + $ZoneMinder::Database::dbh->commit(); return "New path ($NewPath) is empty."; } elsif ( ! -e $NewPath ) { + $ZoneMinder::Database::dbh->commit(); return "New path $NewPath does not exist."; } elsif ( ! -e $OldPath ) { + $ZoneMinder::Database::dbh->commit(); return "Old path $OldPath does not exist."; } ( $NewPath ) = ( $self->Path(undef) =~ /^(.*)$/ ); # De-taint if ( $NewPath eq $OldPath ) { + $ZoneMinder::Database::dbh->commit(); return "New path and old path are the same! $NewPath"; } Debug("Moving event $$self{Id} from $OldPath to $NewPath"); @@ -476,14 +489,22 @@ sub MoveTo { last; } } # end foreach file. - return $error if $error; + + if ( $error ) { + $ZoneMinder::Database::dbh->commit(); + return $error; + } # Succeeded in copying all files, so we may now update the Event. $$self{StorageId} = $$NewStorage{Id}; $$self{Storage} = $NewStorage; $error .= $self->save(); - return $error if $error; + if ( $error ) { + $ZoneMinder::Database::dbh->commit(); + return $error; + } $self->delete_files( $OldStorage ); + $ZoneMinder::Database::dbh->commit(); return $error; } # end sub MoveTo diff --git a/scripts/ZoneMinder/lib/ZoneMinder/Object.pm b/scripts/ZoneMinder/lib/ZoneMinder/Object.pm index f8661313e..df7118f0f 100644 --- a/scripts/ZoneMinder/lib/ZoneMinder/Object.pm +++ b/scripts/ZoneMinder/lib/ZoneMinder/Object.pm @@ -105,6 +105,44 @@ sub load { } # end if } # end sub load +sub lock_and_load { + my ( $self ) = @_; + my $type = ref $self; + + no strict 'refs'; + my $table = ${$type.'::table'}; + if ( ! $table ) { + Error( 'NO table for type ' . $type ); + return; + } # end if + my $primary_key = ${$type.'::primary_key'}; + if ( ! $primary_key ) { + Error( 'NO primary_key for type ' . $type ); + return; + } # end if + + if ( ! $$self{$primary_key} ) { + my ( $caller, undef, $line ) = caller; + Error( (ref $self) . "::lock_and_load called without $primary_key from $caller:$line"); + return; + + } + #$log->debug("Object::load Loading from db $type"); + Debug("Loading $type from $table WHERE $primary_key = $$self{$primary_key}"); + my $data = $ZoneMinder::Database::dbh->selectrow_hashref( "SELECT * FROM $table WHERE $primary_key=? FOR UPDATE", {}, $$self{$primary_key} ); + if ( ! $data ) { + if ( $ZoneMinder::Database::dbh->errstr ) { + Error( "Failure to load Object record for $$self{$primary_key}: Reason: " . $ZoneMinder::Database::dbh->errstr ); + } else { + Debug("No Results Loading $type from $table WHERE $primary_key = $$self{$primary_key}"); + } # end if + } # end if + if ( $data and %$data ) { + @$self{keys %$data} = values %$data; + } # end if +} # end sub lock_and_load + + sub AUTOLOAD { my ( $self, $newvalue ) = @_; my $type = ref($_[0]); diff --git a/scripts/zmfilter.pl.in b/scripts/zmfilter.pl.in index 4db95e58b..dc54a3091 100644 --- a/scripts/zmfilter.pl.in +++ b/scripts/zmfilter.pl.in @@ -320,8 +320,12 @@ sub checkFilter { if ( $filter->{UpdateDiskSpace} ) { my $Event = new ZoneMinder::Event( $$event{Id}, $event ); + $ZoneMinder::Database::dbh->begin_work(); + $Event->lock_and_load(); + $Event->DiskSpace(undef); $Event->save(); + $ZoneMinder::Database::dbh->commit(); } # end if UpdateDiskSpace } # end foreach event } From 1dbd8405a4534f298e3498a771cd9d1b3f09f1b1 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 18 Jan 2018 11:39:09 -0500 Subject: [PATCH 3/5] restructure indexes on the various Events tables. Use two separate indexes for MonitorId and StartTime because we access these tables using one or the other, but rarely both --- db/zm_create.sql.in | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/db/zm_create.sql.in b/db/zm_create.sql.in index 0f70fdcc6..c8b435df2 100644 --- a/db/zm_create.sql.in +++ b/db/zm_create.sql.in @@ -224,7 +224,8 @@ CREATE TABLE `Events_Hour` ( `StartTime` datetime default NULL, `DiskSpace` bigint unsigned default NULL, PRIMARY KEY (`EventId`), - KEY `Events_Hour_MonitorId_StartTime_idx` (`MonitorId`,`StartTime`) + KEY `Events_Hour_MonitorId_idx` (`MonitorId`) + KEY `Events_Hour_StartTime_idx` (`StartTime`) ) ENGINE=@ZM_MYSQL_ENGINE@; DROP TABLE IF EXISTS `Events_Day`; @@ -234,7 +235,8 @@ CREATE TABLE `Events_Day` ( `StartTime` datetime default NULL, `DiskSpace` bigint unsigned default NULL, PRIMARY KEY (`EventId`), - KEY `Events_Day_MonitorId_StartTime_idx` (`MonitorId`,`StartTime`) + KEY `Events_Day_MonitorId_idx` (`MonitorId`) + KEY `Events_Day_StartTime_idx` (`StartTime`) ) ENGINE=@ZM_MYSQL_ENGINE@; @@ -245,7 +247,8 @@ CREATE TABLE `Events_Week` ( `StartTime` datetime default NULL, `DiskSpace` bigint unsigned default NULL, PRIMARY KEY (`EventId`), - KEY `Events_Week_MonitorId_StartTime_idx` (`MonitorId`,`StartTime`) + KEY `Events_Week_MonitorId_idx` (`MonitorId`) + KEY `Events_Week_StartTime_idx` (`StartTime`) ) ENGINE=@ZM_MYSQL_ENGINE@; DROP TABLE IF EXISTS `Events_Month`; @@ -255,7 +258,8 @@ CREATE TABLE `Events_Month` ( `StartTime` datetime default NULL, `DiskSpace` bigint unsigned default NULL, PRIMARY KEY (`EventId`), - KEY `Events_Month_MonitorId_StartTime_idx` (`MonitorId`,`StartTime`) + KEY `Events_Month_MonitorId_idx` (`MonitorId`) + KEY `Events_Month_StartTime_idx` (`StartTime`) ) ENGINE=@ZM_MYSQL_ENGINE@; @@ -265,7 +269,7 @@ CREATE TABLE `Events_Archived` ( `MonitorId` int(10) unsigned NOT NULL, `DiskSpace` bigint unsigned default NULL, PRIMARY KEY (`EventId`), - KEY `Events_Month_MonitorId_idx` (`MonitorId`) + KEY `Events_Archived_MonitorId_idx` (`MonitorId`) ) ENGINE=@ZM_MYSQL_ENGINE@; From 314390a161eb86365c2f1706d259d52f7921a1f4 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 18 Jan 2018 11:39:23 -0500 Subject: [PATCH 4/5] restructure indexes on the various Events tables. Use two separate indexes for MonitorId and StartTime because we access these tables using one or the other, but rarely both --- db/zm_update-1.31.24.sql | 61 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 db/zm_update-1.31.24.sql diff --git a/db/zm_update-1.31.24.sql b/db/zm_update-1.31.24.sql new file mode 100644 index 000000000..9f7bad11c --- /dev/null +++ b/db/zm_update-1.31.24.sql @@ -0,0 +1,61 @@ + +set @exist := (select count(*) from information_schema.statistics where table_name = 'Events_Hour' and index_name = 'Events_Hour_MonitorId_StartTime_idx' and table_schema = database()); +set @sqlstmt := if( @exist > 0, 'DROP INDEX Events_Hour_MonitorId_StartTime_idx ON Events_Hour', "SELECT 'Events_Hour_MonitorId_StartTime_idx INDEX is already removed.'"); +PREPARE stmt FROM @sqlstmt; +EXECUTE stmt; + +set @exist := (select count(*) from information_schema.statistics where table_name = 'Events_Hour' and index_name = 'Events_Hour_MonitorId_idx' and table_schema = database()); +set @sqlstmt := if( @exist > 0, "SELECT 'Events_Hour_MonitorId_idx INDEX already exists.'", "CREATE INDEX `Events_Hour_MonitorId_idx` ON `Events_Hour` (`MonitorId`)"); +PREPARE stmt FROM @sqlstmt; +EXECUTE stmt; + +set @exist := (select count(*) from information_schema.statistics where table_name = 'Events_Hour' and index_name = 'Events_Hour_StartTime_idx' and table_schema = database()); +set @sqlstmt := if( @exist > 0, "SELECT 'Events_Hour_StartTime_idx INDEX already exists.'", "CREATE INDEX `Events_Hour_StartTime_idx` ON `Events_Hour` (`StartTime`)"); +PREPARE stmt FROM @sqlstmt; +EXECUTE stmt; + +set @exist := (select count(*) from information_schema.statistics where table_name = 'Events_Day' and index_name = 'Events_Day_MonitorId_StartTime_idx' and table_schema = database()); +set @sqlstmt := if( @exist > 0, 'DROP INDEX Events_Day_MonitorId_StartTime_idx ON Events_Day', "SELECT 'Events_Day_MonitorId_StartTime_idx INDEX is already removed.'"); +PREPARE stmt FROM @sqlstmt; +EXECUTE stmt; + +set @exist := (select count(*) from information_schema.statistics where table_name = 'Events_Day' and index_name = 'Events_Day_MonitorId_idx' and table_schema = database()); +set @sqlstmt := if( @exist > 0, "SELECT 'Events_Day_MonitorId_idx INDEX already exists.'", "CREATE INDEX `Events_Day_MonitorId_idx` ON `Events_Day` (`MonitorId`)"); +PREPARE stmt FROM @sqlstmt; +EXECUTE stmt; + +set @exist := (select count(*) from information_schema.statistics where table_name = 'Events_Day' and index_name = 'Events_Day_StartTime_idx' and table_schema = database()); +set @sqlstmt := if( @exist > 0, "SELECT 'Events_Day_StartTime_idx INDEX already exists.'", "CREATE INDEX `Events_Day_StartTime_idx` ON `Events_Day` (`StartTime`)"); +PREPARE stmt FROM @sqlstmt; +EXECUTE stmt; + +set @exist := (select count(*) from information_schema.statistics where table_name = 'Events_Week' and index_name = 'Events_Week_MonitorId_StartTime_idx' and table_schema = database()); +set @sqlstmt := if( @exist > 0, 'DROP INDEX Events_Week_MonitorId_StartTime_idx ON Events_Week', "SELECT 'Events_Week_MonitorId_StartTime_idx INDEX is already removed.'"); +PREPARE stmt FROM @sqlstmt; +EXECUTE stmt; + +set @exist := (select count(*) from information_schema.statistics where table_name = 'Events_Week' and index_name = 'Events_Week_MonitorId_idx' and table_schema = database()); +set @sqlstmt := if( @exist > 0, "SELECT 'Events_Week_MonitorId_idx INDEX already exists.'", "CREATE INDEX `Events_Week_MonitorId_idx` ON `Events_Day` (`MonitorId`)"); +PREPARE stmt FROM @sqlstmt; +EXECUTE stmt; + +set @exist := (select count(*) from information_schema.statistics where table_name = 'Events_Week' and index_name = 'Events_Week_StartTime_idx' and table_schema = database()); +set @sqlstmt := if( @exist > 0, "SELECT 'Events_Week_StartTime_idx INDEX already exists.'", "CREATE INDEX `Events_Week_StartTime_idx` ON `Events_Week` (`StartTime`)"); +PREPARE stmt FROM @sqlstmt; +EXECUTE stmt; + + +set @exist := (select count(*) from information_schema.statistics where table_name = 'Events_Month' and index_name = 'Events_Month_MonitorId_StartTime_idx' and table_schema = database()); +set @sqlstmt := if( @exist > 0, 'DROP INDEX Events_Month_MonitorId_StartTime_idx ON Events_Month', "SELECT 'Events_Month_MonitorId_StartTime_idx INDEX is already removed.'"); +PREPARE stmt FROM @sqlstmt; +EXECUTE stmt; + +set @exist := (select count(*) from information_schema.statistics where table_name = 'Events_Month' and index_name = 'Events_Month_MonitorId_idx' and table_schema = database()); +set @sqlstmt := if( @exist > 0, "SELECT 'Events_Month_MonitorId_idx INDEX already exists.'", "CREATE INDEX `Events_Month_MonitorId_idx` ON `Events_Month` (`MonitorId`)"); +PREPARE stmt FROM @sqlstmt; +EXECUTE stmt; + +set @exist := (select count(*) from information_schema.statistics where table_name = 'Events_Month' and index_name = 'Events_Month_StartTime_idx' and table_schema = database()); +set @sqlstmt := if( @exist > 0, "SELECT 'Events_Month_StartTime_idx INDEX already exists.'", "CREATE INDEX `Events_Month_StartTime_idx` ON `Events_Month` (`StartTime`)"); +PREPARE stmt FROM @sqlstmt; +EXECUTE stmt; From 59a127230735c95abebcc6d25a49d58695f2f7e6 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Thu, 18 Jan 2018 11:39:33 -0500 Subject: [PATCH 5/5] bump version --- src/zm_monitor.cpp | 9 ++++++--- version | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/zm_monitor.cpp b/src/zm_monitor.cpp index 928b253e0..dc097d1ab 100644 --- a/src/zm_monitor.cpp +++ b/src/zm_monitor.cpp @@ -793,6 +793,7 @@ uint32_t Monitor::GetLastEventId() const { // This function is crap. double Monitor::GetFPS() const { + // last_write_index is the last capture index. It starts as == image_buffer_count so that the first asignment % image_buffer_count = 0; int index1 = shared_data->last_write_index; if ( index1 == image_buffer_count ) { // last_write_index only has this value on startup before capturing anything. @@ -801,6 +802,7 @@ double Monitor::GetFPS() const { Snapshot *snap1 = &image_buffer[index1]; if ( !snap1->timestamp || !snap1->timestamp->tv_sec ) { // This should be impossible + Warning("Impossible situation. No timestamp on captured image"); return 0.0; } struct timeval time1 = *snap1->timestamp; @@ -810,9 +812,11 @@ double Monitor::GetFPS() const { Snapshot *snap2 = &image_buffer[index2]; // the timestamp pointers are initialized on connection, so that's redundant // tv_sec is probably only zero during the first loop of capturing, so this basically just counts the unused images. - while ( !snap2->timestamp || !snap2->timestamp->tv_sec ) { + // The problem is that there is no locking, and we set the timestamp before we set last_write_index, + // so there is a small window where the next image can have a timestamp in the future + while ( !snap2->timestamp || !snap2->timestamp->tv_sec || tvDiffSec(*snap2->timestamp, *snap1->timestamp) < 0 ) { if ( index1 == index2 ) { - // We didn't find any initialized images + // All images are uncaptured return 0.0; } index2 = (index2+1)%image_buffer_count; @@ -822,7 +826,6 @@ double Monitor::GetFPS() const { struct timeval time2 = *snap2->timestamp; double time_diff = tvDiffSec( time2, time1 ); - double curr_fps = image_count/time_diff; if ( curr_fps < 0.0 ) { diff --git a/version b/version index c9996cab5..c90269192 100644 --- a/version +++ b/version @@ -1 +1 @@ -1.31.23 +1.31.24