Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory issues in ScrollView::MessageReceiver #3872

Merged
merged 3 commits into from
Jul 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
59 changes: 21 additions & 38 deletions src/viewer/scrollview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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];
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.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();
Expand Down Expand Up @@ -319,38 +319,32 @@ 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();
// Check every table entry if it is valid and not already processed.

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();
}
Expand All @@ -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();
Expand All @@ -383,9 +376,6 @@ ScrollView::~ScrollView() {
}
delete semaphore_;
delete points_;
for (auto &i : event_table_) {
delete i;
}
#endif // !GRAPHICS_DISABLED
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}

Expand Down
7 changes: 4 additions & 3 deletions 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 @@ -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.
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 Expand Up @@ -413,7 +414,7 @@ class TESS_API ScrollView {
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_;
Expand Down