MantisBT - Doomseeker
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0004431Doomseeker[All Projects] Bugpublic2024-10-22 18:272024-10-23 15:38
ReporterBlzut3 
Assigned ToZalewa 
PrioritynormalSeverityminorReproducibilitysometimes
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version1.4.1 
Target Version1.5.0Fixed in Version1.5.0 
Summary0004431: IP2C occasionally fails country lookup on startup
DescriptionQuoting the contents of relevant Blzut3 comments for 0004421:

> Am I crazy or are there regressions with the IP2C handling? On 1.4.1 most servers are identified and Doomseeker starts querying instantly. With the latest git Doomseeker hangs completely unresponsive for several seconds at start up and then most servers appear to fail country lookup?

> It does appear the IP2C issue is timing related. Making sure the IP2C parsing doesn't finish until after the query on startup is finished seems to make everything resolve correctly. Presumably something is going wrong if the IP2C parsing finishes mid refresh.

> My working theory right now is that the sequence of events looks something like this:

* Startup refresh triggers.
* Master server responds, server queries begin.
* IP2C finishes parsing.
* Something in main thread happens here that causes the event queue to be flushed

> The root cause of the IP2C issue. (Note line numbers in stack trace are probably nonsense given all the measurement code I added to trace this down in the first place.)

    0x00005555558582f2 in IP2CLoader::~IP2CLoader (this=0x555556006ed0, __in_chrg=<optimized out>) at /home/blzut3/Code/Doomseeker/src/core/ip2c/ip2cloader.cpp:114
    0x00005555557f2353 in MainWindow::ip2cJobsFinished (this=0x555556006ea0) at /home/blzut3/Code/Doomseeker/src/core/gui/mainwindow.cpp:962
    0x00005555556e03a0 in MainWindow::qt_static_metacall (_o=0x555556006ea0, _c=QMetaObject::InvokeMetaMethod, _id=46, _a=0x7fffffffce60)
        at /home/blzut3/Code/Doomseeker/build-qt5/src/core/doomseeker_autogen/DMHXEJ42XS/moc_mainwindow.cpp:327
    0x00007ffff6924862 in ?? () from /lib/x86_64-linux-gnu/libQt5Core.so.5
    0x00005555556e6253 in IP2CLoader::finished (this=0x5555560e80a0) at /home/blzut3/Code/Doomseeker/build-qt5/src/core/doomseeker_autogen/6Q53HC4DWL/moc_ip2cloader.cpp:179
    0x00005555558592c3 in IP2CLoader::ip2cJobsFinished (this=0x5555560e80a0) at /home/blzut3/Code/Doomseeker/src/core/ip2c/ip2cloader.cpp:271
    0x0000555555858684 in IP2CLoader::onUpdateNeeded (this=0x5555560e80a0, status=0) at /home/blzut3/Code/Doomseeker/src/core/ip2c/ip2cloader.cpp:162
    0x000055555585aeb0 in QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<int>, void, void (IP2CLoader::*)(int)>::call (
        f=(void (IP2CLoader::*)(IP2CLoader * const, int)) 0x555555858568 <IP2CLoader::onUpdateNeeded(int)>, o=0x5555560e80a0, arg=0x7fffffffd0d0) at /usr/include/x86_64-linux-gnu/qt5/QtCore/qobjectdefs_impl.h:152
    0x000055555585ac01 in QtPrivate::FunctionPointer<void (IP2CLoader::*)(int)>::call<QtPrivate::List<int>, void> (
        f=(void (IP2CLoader::*)(IP2CLoader * const, int)) 0x555555858568 <IP2CLoader::onUpdateNeeded(int)>, o=0x5555560e80a0, arg=0x7fffffffd0d0) at /usr/include/x86_64-linux-gnu/qt5/QtCore/qobjectdefs_impl.h:185
    0x000055555585a8f4 in QtPrivate::QSlotObject<void (IP2CLoader::*)(int), QtPrivate::List<int>, void>::impl (which=1, this_=0x5555561e40b0, r=0x5555560e80a0, a=0x7fffffffd0d0, ret=0x0)
        at /usr/include/x86_64-linux-gnu/qt5/QtCore/qobjectdefs_impl.h:418
    0x00007ffff692488e in ?? () from /lib/x86_64-linux-gnu/libQt5Core.so.5
    0x00005555556e6717 in IP2CUpdater::updateNeeded (this=0x5555560c0970, _t1=0) at /home/blzut3/Code/Doomseeker/build-qt5/src/core/doomseeker_autogen/6Q53HC4DWL/moc_ip2cupdater.cpp:186
    0x000055555585d85a in IP2CUpdater::checksumDownloadFinished (this=0x5555560c0970) at /home/blzut3/Code/Doomseeker/src/core/ip2c/ip2cupdater.cpp:92


> The IP2CLoader::finished signal is emitted in multiple places, which connects to MainWindow::ip2cJobsFinished. Depending on the specific timing of queued slot execution, this can delete the IP2CLoader mid load and that causes the thread's signals to be disconnected.
Steps To Reproduce
Additional InformationReported originally in comment 0004421:0024060 for 0004421
TagsNo tags attached.
Relationships
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2024-10-22 18:27ZalewaNew Issue
2024-10-22 18:27ZalewaStatusnew => assigned
2024-10-22 18:27ZalewaAssigned To => Zalewa
2024-10-22 18:27ZalewaReporterZalewa => Blzut3
2024-10-22 19:11ZalewaNote Added: 0024080
2024-10-22 19:11ZalewaStatusassigned => needs testing
2024-10-23 00:21Blzut3Note Added: 0024081
2024-10-23 15:38ZalewaNote Added: 0024082
2024-10-23 15:38ZalewaStatusneeds testing => resolved
2024-10-23 15:38ZalewaFixed in Version => 1.5.0
2024-10-23 15:38ZalewaResolutionopen => fixed

Notes
(0024080)
Zalewa   
2024-10-22 19:11   
Everytime I write multithreaded code it has to come up riddled with race conditions :(

I identified one in how the "are we done" condition was checked:
'https://bitbucket.org/Doomseeker/doomseeker/commits/dbc82fb498450aaab1ccfde1cc42be2d47ddc285 [^]'

Blzut, could you apply this and check if the '?' flag problem persists?
(0024081)
Blzut3   
2024-10-23 00:21   
Seems to, and I think that logic change sounds right. I'm fairly sure you could also have just removed the isRunning check and used the parserThread pointer alone (you ultimately set the new flag to false in the same places you set parserThread to nullptr). So the effective change is you're waiting for the main event loop to acknowledge that the thread completed rather than checking the thread's status directly.

On that note if it makes you feel better, the problem was less your multithreaded code had a data race, but rather just being async is enough in this case. (Which is to say the important part is that removing the parsingInProgress/isParsing() flag needs to occur atomically with handling the parsing results with how things are written currently. If other queued calls could happen in between you could have the same problem single threaded.)
(0024082)
Zalewa   
2024-10-23 15:38   
Quote from Blzut3
Just being async is enough

Yeah, that's true. Anyway, I'm glad it was possible to fix this with a simple change.