From bd7e57bd9e22e71d3b413aa499ff3d7cc9bbd647 Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Mon, 18 Jul 2022 12:53:35 +0300 Subject: [PATCH] viewer: Use std::unique_ptr in waiting_for_events data structure The current usage of waiting_for_events is taking ownership of SVEvent pointer from a unique_ptr. This is error prone as all code paths using waiting_for_events need to ensure deletion. We fix it by using unique_ptr in waiting_for_events and all dependent code paths. --- src/classify/intproto.cpp | 4 +--- src/classify/shapeclassifier.cpp | 4 +--- src/textord/ccnontextdetect.cpp | 2 +- src/textord/colfind.cpp | 5 ++--- src/textord/strokewidth.cpp | 2 +- src/training/common/mastertrainer.cpp | 4 +--- src/training/unicharset/lstmtrainer.cpp | 2 +- src/viewer/scrollview.cpp | 27 ++++++++++--------------- src/viewer/scrollview.h | 3 ++- 9 files changed, 21 insertions(+), 32 deletions(-) diff --git a/src/classify/intproto.cpp b/src/classify/intproto.cpp index 373a967257..0638e59907 100644 --- a/src/classify/intproto.cpp +++ b/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 */ diff --git a/src/classify/shapeclassifier.cpp b/src/classify/shapeclassifier.cpp index da0ddb3bc1..11a6168e84 100644 --- a/src/classify/shapeclassifier.cpp +++ b/src/classify/shapeclassifier.cpp @@ -115,7 +115,6 @@ void ShapeClassifier::DebugDisplay(const TrainingSample &sample, Image page_pix, std::vector results; // Debug classification until the user quits. const UNICHARSET &unicharset = GetUnicharset(); - SVEvent *ev; SVEventType ev_type; do { std::vector 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; diff --git a/src/textord/ccnontextdetect.cpp b/src/textord/ccnontextdetect.cpp index 9fb2fd7294..26ae1656fb 100644 --- a/src/textord/ccnontextdetect.cpp +++ b/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 } diff --git a/src/textord/colfind.cpp b/src/textord/colfind.cpp index 9e8558bfd3..3e97c2858e 100644 --- a/src/textord/colfind.cpp +++ b/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 diff --git a/src/textord/strokewidth.cpp b/src/textord/strokewidth.cpp index 8cf23db19c..d8f52f609c 100644 --- a/src/textord/strokewidth.cpp +++ b/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); } diff --git a/src/training/common/mastertrainer.cpp b/src/training/common/mastertrainer.cpp index c45960bded..05b54b5dc7 100644 --- a/src/training/common/mastertrainer.cpp +++ b/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 diff --git a/src/training/unicharset/lstmtrainer.cpp b/src/training/unicharset/lstmtrainer.cpp index 500cea76e8..0ebad4de1b 100644 --- a/src/training/unicharset/lstmtrainer.cpp +++ b/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. diff --git a/src/viewer/scrollview.cpp b/src/viewer/scrollview.cpp index 3e3517b638..087dabe8ec 100644 --- a/src/viewer/scrollview.cpp +++ b/src/viewer/scrollview.cpp @@ -56,8 +56,8 @@ struct SVPolyLineBuffer { static std::map 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> - waiting_for_events; +static std::map, + std::pair>> waiting_for_events; static std::mutex *waiting_for_events_mu; SVEvent *SVEvent::copy() const { @@ -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.release(); + 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.release(); + 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.release(); + 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(); @@ -367,8 +367,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(); @@ -442,19 +441,19 @@ void ScrollView::SetEvent(const SVEvent *svevent) { /// 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 ScrollView::AwaitEvent(SVEventType type) { // Initialize the waiting semaphore. auto *sem = new SVSemaphore(); std::pair ea(this, type); waiting_for_events_mu->lock(); - waiting_for_events[ea] = std::pair(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 +733,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; } diff --git a/src/viewer/scrollview.h b/src/viewer/scrollview.h index a11ebed91c..f583eba579 100644 --- a/src/viewer/scrollview.h +++ b/src/viewer/scrollview.h @@ -36,6 +36,7 @@ #include #include +#include #include namespace tesseract { @@ -186,7 +187,7 @@ class TESS_API ScrollView { void AddEventHandler(SVEventHandler *listener); // Block until an event of the given type is received. - SVEvent *AwaitEvent(SVEventType type); + std::unique_ptr AwaitEvent(SVEventType type); /******************************************************************************* * Getters and Setters