Zandronum Chat on our Discord Server Get the latest version: 3.1
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0004421Doomseeker[All Projects] Epicpublic2024-10-09 20:482024-11-02 22:25
ReporterZalewa 
Assigned ToZalewa 
PrioritynormalSeverityfeatureReproducibilityN/A
StatusassignedResolutionopen 
PlatformOSOS Version
Product Version1.4.1 
Target Version1.5.0Fixed in Version 
Summary0004421: Doomseeker 1.5.0 release
DescriptionLet's release Doomseeker 1.5.0. The detailed instructions are in the checklist attachment, as usual.
Additional InformationDoomseeker release 1.4.1: 0004109
Doomseeker release 1.4.0: 0004082
Attached Filespng file icon Ip2CRegression.png [^] (429,343 bytes) 2024-10-14 03:23
txt file icon release-checklist-1.5-v2.txt [^] (2,792 bytes) 2024-10-27 15:31 [Show Content]

- Relationships

-  Notes
User avatar (0024060)
Blzut3 (administrator)
2024-10-14 03:22

Fixed Qt6 build issues. It appears that the std::sort that crashed was because the return values were inverted.

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?
User avatar (0024062)
Zalewa (developer)
2024-10-14 19:22

The V3 of the IP2C database format was in 1.4.1 already, and I don't see any changes to this code after that. Maybe there's some parser-related timing issue that I'm lucky enough not to hit? The parsing is still happening in a background thread here.

Or maybe the IP2C dat file was mangled?

I've just now deployed a new version of the IP2C database. See if the problem persists after you update. For me it works correctly. I moved the previous .dat file to the "ip2c/old/2023-03-27" directory on the website's server, just in case.

As for the hang: I have noticed a freeze after you press the server refresh button and right before the server table is filled with Zandronum servers. There being 550 Zandronum servers doesn't help with the Qt table performance, but even with Odamex's 180 servers there is a brief but noticeable freeze. This thing could be looked into.
User avatar (0024063)
Blzut3 (administrator)
2024-10-14 22:34
edited on: 2024-10-14 22:35

The hang you notice may indeed be the same, I've traced the change to 8e0d117c8e40f4dccac5c9810ff969ef24dac67d, which seems curious but it does seem to be related to regular expression processing for each of the table rows. Subset of the backtrace I got from breaking while hung:
0x00007ffff678ac7d in QRegularExpression::isValid() const () from /lib/x86_64-linux-gnu/libQt5Core.so.5
0x00007ffff677ee7e in QString::replace(QRegularExpression const&, QString const&) () from /lib/x86_64-linux-gnu/libQt5Core.so.5
0x000055555589bc45 in EnginePlugin::nameCanonical (this=0x555555e6a240) at /home/blzut3/Code/Doomseeker/src/core/plugins/engineplugin.cpp:298
0x000055555589fb99 in PluginLoader::pluginIndexFromName (this=0x555555e6f080, name=...) at /home/blzut3/Code/Doomseeker/src/core/plugins/pluginloader.cpp:317
0x000055555571c6b0 in CustomServerInfo::fromServer (server=0x555556248470) at /home/blzut3/Code/Doomseeker/src/core/customservers.cpp:38
0x00005555558017c4 in ServerListModel::findSameServer (this=0x5555562bcad0, server=0x5555561dd820) at /home/blzut3/Code/Doomseeker/src/core/gui/models/serverlistmodel.cpp:69
0x000055555581bff0 in ServerList::registerServer (this=0x555556349610, server=...) at /home/blzut3/Code/Doomseeker/src/core/gui/serverlist.cpp:400
0x00005555557ebdb4 in MainWindow::finishedQueryingMaster (this=0x555555f859a0, master=0x555555f9a1e0) at /home/blzut3/Code/Doomseeker/src/core/gui/mainwindow.cpp:697


So that's definitely a regression to look at.

Still trying to figure out what the deal is with IP2C. It might not be a regression strictly speaking as rebuilding 1.4.1 seems to be broken for me (even built in my Ubuntu 18.04 chroot), but my deb package still works fine. They're both parsing the same file.

User avatar (0024064)
Blzut3 (administrator)
2024-10-15 01:27
edited on: 2024-10-15 02:39

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.

As for 1.4.1 deb working, it seems Release build related. So if it parses fast enough things seem to be OK too. It does seem to hang issue makes it considerably more likely to get into this state, but hard to say.

Edit: Looks like the cause is IP2CLoader::onParsingFinished is never called. Haven't figured out why it works in some cases but not others.

User avatar (0024065)
Zalewa (developer)
2024-10-15 16:32
edited on: 2024-10-15 16:33

Running a regexp in a should-be-light-weight function was probably not the best idea. I just moved the piece that derives the canonical name, if the plugin didn't specify one, to the init() method.

'https://bitbucket.org/Doomseeker/doomseeker/commits/e686748435ec4a1550b6ae0ca658b8aced89da1f [^]'

This has sped things up noticeably already, but there's still something off.

Adding a qDebug() with a static counter to EnginePlugin::nameCanonical() and refreshing Chocolate Doom servers has logged that the method was called roughly 2400 times. Chocolate Doom has 17 servers, and 4 of them don't respond...

User avatar (0024066)
Zalewa (developer)
2024-10-15 18:29

Another optimization that avoids calling gPlugins->pluginIndexFromName():

'https://bitbucket.org/Doomseeker/doomseeker/commits/c070ba22c5300f5575bca901121f7a45691854cf [^]'

Plus it sits entirely in the header, allowing the compiler to do its own optimizations.

I think that there may still be some room for improvement here. Maybe the server identity could be precalculated into an int hash code, and this could be compared first before the exact details are compared?
User avatar (0024067)
Blzut3 (administrator)
2024-10-15 22:46

Confirming that these optimizations are enough to fix the hang for me.
User avatar (0024069)
Zalewa (developer)
2024-10-19 11:44
edited on: 2024-10-19 11:45

I can't reproduce the IP2C loading problem. Even if I introduce an artificial sleep to the IP2CParsingThread, it still finishes correctly and the flags do eventually change from '?' to proper country flags. I tried enabling all plugins, all server refresh on startup, and IP2C update check on startup, and I tried Doomseeker both before and after the server table performance fixes.

The onParsingFinished() slot is tied to QThread::finished() signal. If it not being called is the problem here, I can see this happening only in two cases:

1. The IP2CParsingThread never finishes, or
2. the signal is disconnected prematurely.

There's one explicit disconnect, but it's in a place that also starts a new parsing thread, so the new thread should emit the signal eventually. Another potential cause may be that the finished() signal is also tied to the thread's QObject::deleteLater(). I'm guessing here, but maybe this causes a disconnect and prevents the signal from reaching the other slot?

If that is the case, then maybe this patch will help:

diff --git a/src/core/ip2c/ip2cloader.cpp b/src/core/ip2c/ip2cloader.cpp
index 3dce9fe1..b6702763 100644
--- a/src/core/ip2c/ip2cloader.cpp
+++ b/src/core/ip2c/ip2cloader.cpp
@@ -272,11 +272,15 @@ void IP2CLoader::ip2cReadDatabase(const QString &filePath)
         d->parserThread->disconnect(this);
     }
 
- d->parserThread = new IP2CParserThread(filePath);
- connect(d->parserThread, &IP2CParserThread::finished,
+ auto parserThread = new IP2CParserThread(filePath);
+ connect(parserThread, &IP2CParserThread::finished,
         this, &IP2CLoader::onParsingFinished);
- connect(d->parserThread, &IP2CParserThread::finished,
- d->parserThread, &QObject::deleteLater);
+ connect(parserThread, &IP2CParserThread::finished,
+ parserThread, [parserThread]() {
+ QTimer::singleShot(0, parserThread, &QObject::deleteLater);
+ });
+
+ d->parserThread = parserThread;
     d->parserThread->start();
 }


User avatar (0024071)
Blzut3 (administrator)
2024-10-19 21:17
edited on: 2024-10-19 21:18

I've commented out the deleteLater connection to no avail so I think I already ruled that out. Similarly setting the onParsingFinished connection to a direct connection works (ignoring possible threading issues), so it's being emitted. But there's a certain sequence of things where it seems the queued connection gets dropped (and using a blocking connection doesn't change anything here).

I'm still completely unsure what's going on but 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

If that something happens fast enough to occur before IP2C parsing finishes, things work. If IP2C parsing takes long enough that it completes after, things work.

User avatar (0024072)
Blzut3 (administrator)
2024-10-20 04:43

Still not sure why the call is being dropped, but one sweep it under the rug solution appears to be to move "d->ip2cLoader->load();" to MainWindow::postInitAppStartup.

Not knowing the root cause is really uncomfortable though. I don't think there's any reason why the main event loop would need to be run first before queued connections work, and that wouldn't explain why it works if it completes well before the event loop starts.
User avatar (0024073)
Zalewa (developer)
2024-10-20 21:07

One last thing I wanted to fix before the release was 0004173, but it's not as trivial as I'd hoped it to be. Unless you have some better idea than what I outlined in 0004173:0024070, Blzut, then I will postpone it for a future release.

For Windows build I will distribute 1.5.0 with Qt 5.7 still, because this is the final version that runs on Windows XP, but I think that it's time to retire Windows XP support after that. I need to consider how I want to deal with the auto-updates here.
User avatar (0024075)
Blzut3 (administrator)
2024-10-21 01:47

I think I finally figured out 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.

Also ACK on the Windows XP note. Do you plan to jump to Qt6 after?
User avatar (0024077)
Zalewa (developer)
2024-10-21 16:02
edited on: 2024-10-21 20:18

Quote
Do you plan to jump to Qt6 after?

Yes, I will update the compiler too. I should probably explore first if there will be compatibility issues with Windows 7 (which will become the new XP now). The docs say that the platform is not supported, even with Qt 6.0, but this doesn't mean that the program won't run ;)

'https://doc.qt.io/archives/qt-6.0/supported-platforms.html#windows [^]'

User avatar (0024078)
Blzut3 (administrator)
2024-10-22 00:02

Well if you end up deciding to stick with Qt 5.15 for Windows 7 that's fine too. Guess it's worth writing some notes on the status for Linux: KDE 6 is just starting to roll out so it missed the LTS cycle. Debian 13 and Ubuntu 26.04 should have a solid Qt6 experience, so ideally I'd like Qt 5.15 supported until then.

Debian 11 with backports repo, Debian 12, and Ubuntu 22.04+ all ship Qt 6.2+. Theming might be a bit raw on them though, which I think is fine if it's not the latest OS release.

Should probably also mention that Ubuntu 20.04 is Qt 5.12, so until May 2025 that's my ideal minimum right now. But at our current cadence that might be irrelevant by the next release.
User avatar (0024079)
Zalewa (developer)
2024-10-22 18:32

I created ticket 0004431 for the IP2C issue, quoting your relevant comments as the description.

Quote from Blzut3
Ubuntu 20.04 is Qt 5.12, so until May 2025 that's my ideal minimum right now.

Once I'm ready to migrate I shall set the Docker images used in Bitbucket's Pipelines to this version then. This is very useful as a build compatibility check harness.
User avatar (0024106)
Zalewa (developer)
2024-10-27 16:32

I did all the steps from "1", except for those where the files are deployed to Doomseeker's website because my write mount is temporarily unavailable. Time for step "2".

Also, I replaced the checklist file with a v2 because the previous checklist still referred to SRB2 plugin, which was dealt with in the previous release. I just uploaded it without reading it carefully.
User avatar (0024114)
Blzut3 (administrator)
2024-11-02 22:25

macOS, Linux, and source packages are uploaded.

Issue Community Support
Only registered users can voice their support. Click here to register, or here to log in.
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-09 20:48 Zalewa New Issue
2024-10-09 20:48 Zalewa File Added: release-checklist-1.5.txt
2024-10-09 20:49 Zalewa Severity minor => feature
2024-10-14 03:22 Blzut3 Note Added: 0024060
2024-10-14 03:23 Blzut3 File Added: Ip2CRegression.png
2024-10-14 19:22 Zalewa Note Added: 0024062
2024-10-14 22:34 Blzut3 Note Added: 0024063
2024-10-14 22:35 Blzut3 Note Edited: 0024063 View Revisions
2024-10-15 01:27 Blzut3 Note Added: 0024064
2024-10-15 01:30 Blzut3 Note Edited: 0024064 View Revisions
2024-10-15 02:39 Blzut3 Note Edited: 0024064 View Revisions
2024-10-15 16:32 Zalewa Note Added: 0024065
2024-10-15 16:33 Zalewa Note Edited: 0024065 View Revisions
2024-10-15 18:29 Zalewa Note Added: 0024066
2024-10-15 22:46 Blzut3 Note Added: 0024067
2024-10-19 11:44 Zalewa Note Added: 0024069
2024-10-19 11:45 Zalewa Note Edited: 0024069 View Revisions
2024-10-19 21:17 Blzut3 Note Added: 0024071
2024-10-19 21:18 Blzut3 Note Edited: 0024071 View Revisions
2024-10-20 04:43 Blzut3 Note Added: 0024072
2024-10-20 21:07 Zalewa Note Added: 0024073
2024-10-21 01:47 Blzut3 Note Added: 0024075
2024-10-21 16:02 Zalewa Note Added: 0024077
2024-10-21 20:18 Zalewa Note Edited: 0024077 View Revisions
2024-10-22 00:02 Blzut3 Note Added: 0024078
2024-10-22 18:32 Zalewa Note Added: 0024079
2024-10-23 17:26 Zalewa Assigned To => Zalewa
2024-10-23 17:26 Zalewa Status new => assigned
2024-10-27 15:31 Zalewa File Added: release-checklist-1.5-v2.txt
2024-10-27 15:31 Zalewa File Deleted: release-checklist-1.5.txt
2024-10-27 16:30 Zalewa Assigned To Zalewa => Blzut3
2024-10-27 16:32 Zalewa Note Added: 0024106
2024-11-02 22:25 Blzut3 Assigned To Blzut3 => Zalewa
2024-11-02 22:25 Blzut3 Note Added: 0024114






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2024 MantisBT Team
Powered by Mantis Bugtracker