Cleanup thread cleaning and fix PinPAD crash #370
Conversation
metsma
force-pushed
the
thread
branch
9 times, most recently
from
a6f2233 to
2911ccf
Compare
metsma
force-pushed
the
thread
branch
3 times, most recently
from
3f0e6c7 to
d9aa4d1
Compare
metsma
force-pushed
the
thread
branch
8 times, most recently
from
ba65333 to
893a5ad
Compare
metsma
force-pushed
the
thread
branch
3 times, most recently
from
085c22a to
6f6b023
Compare
Fixes #346 Signed-off-by: Raul Metsma <raul@metsma.ee>
| ControllerChildThread::waitForControllerNotify.wakeAll(); | ||
| while (thread->isRunning()) { | ||
| using namespace std::chrono_literals; | ||
| thread->wait(100ms); |
There was a problem hiding this comment.
The 100 millisecond wait made only sense along with event processing, recommend to add it back:
| thread->wait(100ms); | |
| thread->wait(100ms); | |
| QCoreApplication::processEvents(); |
Otherwise the while loop does not have a purpose, it is equivalent to just waiting indefinitely.
There was a problem hiding this comment.
All calls waitForChildThreads() are after disposeUI, no point to process any more events.
and timer makes sure that function does not continue before all thread are ended
| void Controller::onDialogCancel() | ||
| { | ||
| REQUIRE_NON_NULL(window) | ||
|
|
There was a problem hiding this comment.
Shouldn't you emit stopCardEventMonitorThread() here?
There was a problem hiding this comment.
I do not see that there can be any running thread at this point
| { | ||
| static MockUI instance; | ||
| return &instance; | ||
| return new MockUI; |
There was a problem hiding this comment.
This leaks, no parent nor WA_DeleteOnClose. Should you add WA_DeleteOnClose?
There was a problem hiding this comment.
I do not think it will ever shown and cosed. Right now there is double free crash
Fixes #346
Signed-off-by: Raul Metsma raul@metsma.ee