Merge pull request #3872 from p12tic/fix-scrollview-double-free (fixes issue #3869)

Fix memory issues in ScrollView::MessageReceiver.

src/classify/intproto.cpp

@@ -1163,12 +1163,11 @@ void FillPPLinearBits(uint32_t ParamTable[NUM_PP_BUCKETS][WERDS_PER_PP_VECTOR],
 CLASS_ID Classify::GetClassToDebug(const char *Prompt, bool *adaptive_on, bool *pretrained_on,
                                    int *shape_id) {
   tprintf("%s\n", Prompt);
-  SVEvent *ev;
   SVEventType ev_type;
   int unichar_id = INVALID_UNICHAR_ID;
   // Wait until a click or popup event.
   do {
-    ev = IntMatchWindow->AwaitEvent(SVET_ANY);
+    auto ev = IntMatchWindow->AwaitEvent(SVET_ANY);
     ev_type = ev->type;
     if (ev_type == SVET_POPUP) {
       if (ev->command_id == IDA_SHAPE_INDEX) {
@@ -1214,7 +1213,6 @@ CLASS_ID Classify::GetClassToDebug(const char *Prompt, bool *adaptive_on, bool *
         }
       }
     }
-    delete ev;
   } while (ev_type != SVET_CLICK);
   return 0;
 } /* GetClassToDebug */

src/classify/shapeclassifier.cpp

@@ -115,7 +115,6 @@ void ShapeClassifier::DebugDisplay(const TrainingSample &sample, Image page_pix,
   std::vector<UnicharRating> results;
   // Debug classification until the user quits.
   const UNICHARSET &unicharset = GetUnicharset();
-  SVEvent *ev;
   SVEventType ev_type;
   do {
     std::vector<ScrollView *> windows;
@@ -135,7 +134,7 @@ void ShapeClassifier::DebugDisplay(const TrainingSample &sample, Image page_pix,
     UNICHAR_ID old_unichar_id;
     do {
       old_unichar_id = unichar_id;
-      ev = debug_win->AwaitEvent(SVET_ANY);
+      auto ev = debug_win->AwaitEvent(SVET_ANY);
       ev_type = ev->type;
       if (ev_type == SVET_POPUP) {
         if (unicharset.contains_unichar(ev->parameter)) {
@@ -144,7 +143,6 @@ void ShapeClassifier::DebugDisplay(const TrainingSample &sample, Image page_pix,
           tprintf("Char class '%s' not found in unicharset", ev->parameter);
         }
       }
-      delete ev;
     } while (unichar_id == old_unichar_id && ev_type != SVET_CLICK && ev_type != SVET_DESTROY);
     for (auto window : windows) {
       delete window;

src/textord/ccnontextdetect.cpp

@@ -134,7 +134,7 @@ Image CCNonTextDetect::ComputeNonTextMask(bool debug, Image photo_map, TO_BLOCK
 #endif // !GRAPHICS_DISABLED
     pixWrite("junkccphotomask.png", pix, IFF_PNG);
 #ifndef GRAPHICS_DISABLED
-    delete win->AwaitEvent(SVET_DESTROY);
+    win->AwaitEvent(SVET_DESTROY);
     delete win;
 #endif // !GRAPHICS_DISABLED
   }

src/textord/colfind.cpp

@@ -440,7 +440,7 @@ int ColumnFinder::FindBlocks(PageSegMode pageseg_mode, Image scaled_color, int s
           DisplayTabVectors(window);
         }
         if (window != nullptr && textord_tabfind_show_partitions > 1) {
-          delete window->AwaitEvent(SVET_DESTROY);
+          window->AwaitEvent(SVET_DESTROY);
         }
       }
     }
@@ -476,7 +476,7 @@ int ColumnFinder::FindBlocks(PageSegMode pageseg_mode, Image scaled_color, int s
     bool waiting = false;
     do {
       waiting = false;
-      SVEvent *event = blocks_win_->AwaitEvent(SVET_ANY);
+      auto event = blocks_win_->AwaitEvent(SVET_ANY);
       if (event->type == SVET_INPUT && event->parameter != nullptr) {
         if (*event->parameter == 'd') {
           result = -1;
@@ -488,7 +488,6 @@ int ColumnFinder::FindBlocks(PageSegMode pageseg_mode, Image scaled_color, int s
       } else {
         waiting = true;
       }
-      delete event;
     } while (waiting);
   }
 #endif // !GRAPHICS_DISABLED

src/textord/strokewidth.cpp

@@ -123,7 +123,7 @@ StrokeWidth::StrokeWidth(int gridsize, const ICOORD &bleft, const ICOORD &tright
 StrokeWidth::~StrokeWidth() {
 #ifndef GRAPHICS_DISABLED
   if (widths_win_ != nullptr) {
-    delete widths_win_->AwaitEvent(SVET_DESTROY);
+    widths_win_->AwaitEvent(SVET_DESTROY);
     if (textord_tabfind_only_strokewidths) {
       exit(0);
     }

src/training/common/mastertrainer.cpp

@@ -785,9 +785,8 @@ void MasterTrainer::DisplaySamples(const char *unichar_str1, int cloud_font,
   ScrollView *s_window = CreateFeatureSpaceWindow("Samples", 100, 500);
   SVEventType ev_type;
   do {
-    SVEvent *ev;
     // Wait until a click or popup event.
-    ev = f_window->AwaitEvent(SVET_ANY);
+    auto ev = f_window->AwaitEvent(SVET_ANY);
     ev_type = ev->type;
     if (ev_type == SVET_CLICK) {
       int feature_index = feature_space.XYToFeatureIndex(ev->x, ev->y);
@@ -801,7 +800,6 @@ void MasterTrainer::DisplaySamples(const char *unichar_str1, int cloud_font,
         s_window->Update();
       }
     }
-    delete ev;
   } while (ev_type != SVET_DESTROY);
 }
 #endif // !GRAPHICS_DISABLED

src/training/unicharset/lstmtrainer.cpp

@@ -891,7 +891,7 @@ Trainability LSTMTrainer::TrainOnLine(const ImageData *trainingdata,
   }
 #ifndef GRAPHICS_DISABLED
   if (debug_interval_ == 1 && debug_win_ != nullptr) {
-    delete debug_win_->AwaitEvent(SVET_CLICK);
+    debug_win_->AwaitEvent(SVET_CLICK);
   }
 #endif // !GRAPHICS_DISABLED
   // Roll the memory of past means.

src/viewer/scrollview.cpp

@@ -56,12 +56,12 @@ struct SVPolyLineBuffer {
 static std::map<int, ScrollView *> svmap;
 static std::mutex *svmap_mu;
 // A map of all semaphores waiting for a specific event on a specific window.
-static std::map<std::pair<ScrollView *, SVEventType>, std::pair<SVSemaphore *, SVEvent *>>
-    waiting_for_events;
+static std::map<std::pair<ScrollView *, SVEventType>,
+                std::pair<SVSemaphore *, std::unique_ptr<SVEvent>>> waiting_for_events;
 static std::mutex *waiting_for_events_mu;
 
-SVEvent *SVEvent::copy() const {
-  auto *any = new SVEvent;
+std::unique_ptr<SVEvent> SVEvent::copy() const {
+  auto any = std::unique_ptr<SVEvent>(new SVEvent);
   any->command_id = command_id;
   any->counter = counter;
   any->parameter = new char[strlen(parameter) + 1];
@@ -158,13 +158,13 @@ void ScrollView::MessageReceiver() {
                                                                     SVET_ANY);
       waiting_for_events_mu->lock();
       if (waiting_for_events.count(awaiting_list) > 0) {
-        waiting_for_events[awaiting_list].second = cur.get();
+        waiting_for_events[awaiting_list].second = std::move(cur);
         waiting_for_events[awaiting_list].first->Signal();
       } else if (waiting_for_events.count(awaiting_list_any) > 0) {
-        waiting_for_events[awaiting_list_any].second = cur.get();
+        waiting_for_events[awaiting_list_any].second = std::move(cur);
         waiting_for_events[awaiting_list_any].first->Signal();
       } else if (waiting_for_events.count(awaiting_list_any_window) > 0) {
-        waiting_for_events[awaiting_list_any_window].second = cur.get();
+        waiting_for_events[awaiting_list_any_window].second = std::move(cur);
         waiting_for_events[awaiting_list_any_window].first->Signal();
       }
       waiting_for_events_mu->unlock();
@@ -319,12 +319,9 @@ void ScrollView::Initialize(const char *name, int x_pos, int y_pos, int x_size,
 
 /// Sits and waits for events on this window.
 void ScrollView::StartEventHandler() {
-  SVEvent *new_event;
-
   for (;;) {
     stream_->Flush();
     semaphore_->Wait();
-    new_event = nullptr;
     int serial = -1;
     int k = -1;
     mutex_.lock();
@@ -332,25 +329,22 @@ void ScrollView::StartEventHandler() {
 
     for (int i = 0; i < SVET_COUNT; i++) {
       if (event_table_[i] != nullptr && (serial < 0 || event_table_[i]->counter < serial)) {
-        new_event = event_table_[i];
         serial = event_table_[i]->counter;
         k = i;
       }
     }
     // If we didn't find anything we had an old alarm and just sleep again.
-    if (new_event != nullptr) {
-      event_table_[k] = nullptr;
+    if (k != -1) {
+      auto new_event = std::move(event_table_[k]);
       mutex_.unlock();
       if (event_handler_ != nullptr) {
-        event_handler_->Notify(new_event);
+        event_handler_->Notify(new_event.get());
       }
       if (new_event->type == SVET_DESTROY) {
         // Signal the destructor that it is safe to terminate.
         event_handler_ended_ = true;
-        delete new_event; // Delete the pointer after it has been processed.
         return;
       }
-      delete new_event; // Delete the pointer after it has been processed.
     } else {
       mutex_.unlock();
     }
@@ -367,8 +361,7 @@ ScrollView::~ScrollView() {
     // So the event handling thread can quit.
     SendMsg("destroy()");
 
-    SVEvent *sve = AwaitEvent(SVET_DESTROY);
-    delete sve;
+    AwaitEvent(SVET_DESTROY);
     svmap_mu->lock();
     svmap[window_id_] = nullptr;
     svmap_mu->unlock();
@@ -383,9 +376,6 @@ ScrollView::~ScrollView() {
   }
   delete semaphore_;
   delete points_;
-  for (auto &i : event_table_) {
-    delete i;
-  }
 #endif // !GRAPHICS_DISABLED
 }
 
@@ -425,36 +415,33 @@ void ScrollView::Signal() {
 
 void ScrollView::SetEvent(const SVEvent *svevent) {
   // Copy event
-  SVEvent *any = svevent->copy();
-  SVEvent *specific = svevent->copy();
+  auto any = svevent->copy();
+  auto specific = svevent->copy();
   any->counter = specific->counter + 1;
 
   // Place both events into the queue.
   std::lock_guard<std::mutex> guard(mutex_);
-  // Delete the old objects..
-  delete event_table_[specific->type];
-  delete event_table_[SVET_ANY];
-  // ...and put the new ones in the table.
-  event_table_[specific->type] = specific;
-  event_table_[SVET_ANY] = any;
+
+  event_table_[specific->type] = std::move(specific);
+  event_table_[SVET_ANY] = std::move(any);
 }
 
 /// Block until an event of the given type is received.
 /// Note: The calling function is responsible for deleting the returned
 /// SVEvent afterwards!
-SVEvent *ScrollView::AwaitEvent(SVEventType type) {
+std::unique_ptr<SVEvent> ScrollView::AwaitEvent(SVEventType type) {
   // Initialize the waiting semaphore.
   auto *sem = new SVSemaphore();
   std::pair<ScrollView *, SVEventType> ea(this, type);
   waiting_for_events_mu->lock();
-  waiting_for_events[ea] = std::pair<SVSemaphore *, SVEvent *>(sem, (SVEvent *)nullptr);
+  waiting_for_events[ea] = {sem, nullptr};
   waiting_for_events_mu->unlock();
   // Wait on it, but first flush.
   stream_->Flush();
   sem->Wait();
   // Process the event we got woken up for (its in waiting_for_events pair).
   waiting_for_events_mu->lock();
-  SVEvent *ret = waiting_for_events[ea].second;
+  auto ret = std::move(waiting_for_events[ea].second);
   waiting_for_events.erase(ea);
   delete sem;
   waiting_for_events_mu->unlock();
@@ -734,23 +721,19 @@ void ScrollView::Brush(Color color) {
 // Shows a modal Input Dialog which can return any kind of String
 char *ScrollView::ShowInputDialog(const char *msg) {
   SendMsg("showInputDialog(\"%s\")", msg);
-  SVEvent *ev;
   // wait till an input event (all others are thrown away)
-  ev = AwaitEvent(SVET_INPUT);
+  auto ev = AwaitEvent(SVET_INPUT);
   char *p = new char[strlen(ev->parameter) + 1];
   strcpy(p, ev->parameter);
-  delete ev;
   return p;
 }
 
 // Shows a modal Yes/No Dialog which will return 'y' or 'n'
 int ScrollView::ShowYesNoDialog(const char *msg) {
   SendMsg("showYesNoDialog(\"%s\")", msg);
-  SVEvent *ev;
   // Wait till an input event (all others are thrown away)
-  ev = AwaitEvent(SVET_INPUT);
+  auto ev = AwaitEvent(SVET_INPUT);
   int a = ev->parameter[0];
-  delete ev;
   return a;
 }
 

src/viewer/scrollview.h

@@ -36,6 +36,7 @@
 #include <tesseract/export.h>
 
 #include <cstdio>
+#include <memory>
 #include <mutex>
 
 namespace tesseract {
@@ -69,7 +70,7 @@ struct SVEvent {
   ~SVEvent() {
     delete[] parameter;
   }
-  SVEvent *copy() const;
+  std::unique_ptr<SVEvent> copy() const;
   SVEventType type = SVET_DESTROY; // What kind of event.
   ScrollView *window = nullptr;    // Window event relates to.
   char *parameter = nullptr;       // Any string that might have been passed as argument.
@@ -186,7 +187,7 @@ public:
   void AddEventHandler(SVEventHandler *listener);
 
   // Block until an event of the given type is received.
-  SVEvent *AwaitEvent(SVEventType type);
+  std::unique_ptr<SVEvent> AwaitEvent(SVEventType type);
 
   /*******************************************************************************
    * Getters and Setters
@@ -413,7 +414,7 @@ private:
   static SVNetwork *stream_;
 
   // Table of all the currently queued events.
-  SVEvent *event_table_[SVET_COUNT];
+  std::unique_ptr<SVEvent> event_table_[SVET_COUNT];
 
   // Mutex to access the event_table_ in a synchronized fashion.
   std::mutex mutex_;