Push locked packets into event packetqueue instead of unlcoked contents. Prevents freeing of data before writing to event, hence crashing.

src/zm_event.cpp

@@ -301,9 +301,9 @@ void Event::updateNotes(const StringSetMap &newNoteSetMap) {
   }  // end if update
 }  // void Event::updateNotes(const StringSetMap &newNoteSetMap)
 
-void Event::AddPacket(const std::shared_ptr<ZMPacket>&packet) {
+void Event::AddPacket(ZMLockedPacket *packetlock) {
   std::unique_lock<std::mutex> lck(packet_queue_mutex);
-  packet_queue.push(packet);
+  packet_queue.push(packetlock);
   packet_queue_condition.notify_one();
 }
 
@@ -684,7 +684,9 @@ void Event::Run() {
   while (true) {
     if (!packet_queue.empty()) {
       Debug(1, "adding packet");
-      this->AddPacket_(packet_queue.front());
+      const ZMLockedPacket * packet_lock = packet_queue.front();
+      this->AddPacket_(packet_lock->packet_);
+      delete packet_lock;
       packet_queue.pop();
     } else {
       if (terminate_ or zm_terminate) {

src/zm_event.h

@@ -105,7 +105,7 @@ class Event {
 
     void createNotes(std::string &notes);
 
-    std::queue<std::shared_ptr<ZMPacket>> packet_queue;
+    std::queue<ZMLockedPacket *> packet_queue;
     std::mutex packet_queue_mutex;
     std::condition_variable packet_queue_condition;
 
@@ -134,7 +134,7 @@ class Event {
     SystemTimePoint EndTime() const { return end_time; }
     TimePoint::duration Duration() const { return end_time - start_time; };
 
-    void AddPacket(const std::shared_ptr<ZMPacket> &p);
+    void AddPacket(ZMLockedPacket *);
     void AddPacket_(const std::shared_ptr<ZMPacket> &p);
     bool WritePacket(const std::shared_ptr<ZMPacket> &p);
     bool SendFrameImage(const Image *image, bool alarm_frame=false);

src/zm_monitor.cpp

@@ -2270,34 +2270,34 @@ bool Monitor::Analyse() {
       shared_data->state = state = IDLE;
     } // end if ( trigger_data->trigger_state != TRIGGER_OFF )
 
-    if (event) event->AddPacket(snap);
+    if (event) event->AddPacket(packet_lock);
+
+    // In the case where people have pre-alarm frames, the web ui will generate the frame images
+    // from the mp4. So no one will notice anyways.
+    if (snap->image and (videowriter == PASSTHROUGH)) {
+      if (!savejpegs) {
+        Debug(1, "Deleting image data for %d", snap->image_index);
+        // Don't need raw images anymore
+        delete snap->image;
+        snap->image = nullptr;
+      }
+      if (snap->analysis_image and !(savejpegs & 2)) {
+        Debug(1, "Deleting analysis image data for %d", snap->image_index);
+        delete snap->analysis_image;
+        snap->analysis_image = nullptr;
+      }
+    }
+
+    packetqueue.clearPackets(snap);
+
+    if (snap->codec_type == AVMEDIA_TYPE_VIDEO) {
+      // Only do these if it's a video packet.
+      shared_data->last_read_index = snap->image_index;
+      analysis_image_count++;
+    }
+    packetqueue.increment_it(analysis_it);
+    if (!event) delete packet_lock;
   } // end scope for event_lock
-
-  // In the case where people have pre-alarm frames, the web ui will generate the frame images
-  // from the mp4. So no one will notice anyways.
-  if (snap->image and (videowriter == PASSTHROUGH)) {
-    if (!savejpegs) {
-      Debug(1, "Deleting image data for %d", snap->image_index);
-      // Don't need raw images anymore
-      delete snap->image;
-      snap->image = nullptr;
-    }
-    if (snap->analysis_image and !(savejpegs & 2)) {
-      Debug(1, "Deleting analysis image data for %d", snap->image_index);
-      delete snap->analysis_image;
-      snap->analysis_image = nullptr;
-    }
-  }
-
-  packetqueue.clearPackets(snap);
-
-  if (snap->codec_type == AVMEDIA_TYPE_VIDEO) {
-    // Only do these if it's a video packet.
-    shared_data->last_read_index = snap->image_index;
-    analysis_image_count++;
-  }
-  packetqueue.increment_it(analysis_it);
-  delete packet_lock;
   //packetqueue.unlock(packet_lock);
   shared_data->last_read_time = std::chrono::system_clock::to_time_t(std::chrono::system_clock::now());
 
@@ -2870,16 +2870,16 @@ Event * Monitor::openEvent(
 
   // Write out starting packets, do not modify packetqueue it will garbage collect itself
   while (starting_packet and ((*start_it) != *analysis_it)) {
-    event->AddPacket(starting_packet);
+    event->AddPacket(starting_packet_lock);
     // Have added the packet, don't want to unlock it until we have locked the next
 
     packetqueue.increment_it(start_it);
     if ((*start_it) == *analysis_it) {
-      if (starting_packet_lock) delete starting_packet_lock;
+      //if (starting_packet_lock) delete starting_packet_lock;
       break;
     }
     ZMLockedPacket *lp = packetqueue.get_packet(start_it);
-    delete starting_packet_lock;
+    //delete starting_packet_lock;
     if (!lp) return nullptr; // only on terminate FIXME
     starting_packet_lock = lp;
     starting_packet = lp->packet_;