Zandronum Chat on our Discord Server Get the latest version: 3.2
Source Code
My View | View Issues | Change Log | Roadmap | Doomseeker Issue Support Ranking | Rules | My Account
  

View Issue Details [ Jump to Notes ] [ Issue History ] [ Print ]
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.
Additional InformationReported originally in comment 0004421:0024060 for 0004421
Attached Files

- Relationships

-  Notes
User avatar (0024080)
Zalewa (developer)
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?
User avatar (0024081)
Blzut3 (administrator)
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.)
User avatar (0024082)
Zalewa (developer)
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.

Issue Community Support
This issue is already marked as resolved.
If you feel that is not the case, please reopen it and explain why.
Supporters: No one explicitly supports this issue yet.
Opponents: No one explicitly opposes this issue yet.

- Issue History
Date Modified Username Field Change
2024-10-22 18:27 Zalewa New Issue
2024-10-22 18:27 Zalewa Status new => assigned
2024-10-22 18:27 Zalewa Assigned To => Zalewa
2024-10-22 18:27 Zalewa Reporter Zalewa => Blzut3
2024-10-22 19:11 Zalewa Note Added: 0024080
2024-10-22 19:11 Zalewa Status assigned => needs testing
2024-10-23 00:21 Blzut3 Note Added: 0024081
2024-10-23 15:38 Zalewa Note Added: 0024082
2024-10-23 15:38 Zalewa Status needs testing => resolved
2024-10-23 15:38 Zalewa Fixed in Version => 1.5.0
2024-10-23 15:38 Zalewa Resolution open => fixed




Questions or other issues? Contact Us.

Links

Copyright © 2000 - 2025 MantisBT Team
Powered by Mantis Bugtracker