Skip to content

Commit

Permalink
viewer: Use std::unique_ptr in waiting_for_events data structure
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
p12tic committed Jul 18, 2022
1 parent e617c6d commit bd7e57b
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 32 deletions.
4 changes: 1 addition & 3 deletions src/classify/intproto.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 */
Expand Down
4 changes: 1 addition & 3 deletions src/classify/shapeclassifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)) {
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/textord/ccnontextdetect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 2 additions & 3 deletions src/textord/colfind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/textord/strokewidth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 1 addition & 3 deletions src/training/common/mastertrainer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/training/unicharset/lstmtrainer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
27 changes: 11 additions & 16 deletions src/viewer/scrollview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ 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 {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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<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();
Expand Down Expand Up @@ -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;
}

Expand Down
3 changes: 2 additions & 1 deletion src/viewer/scrollview.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <tesseract/export.h>

#include <cstdio>
#include <memory>
#include <mutex>

namespace tesseract {
Expand Down Expand Up @@ -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<SVEvent> AwaitEvent(SVEventType type);

/*******************************************************************************
* Getters and Setters
Expand Down

0 comments on commit bd7e57b

Please sign in to comment.