Bugfix crash on port change
ClosedPublic

Authored by trufanov on May 22 2018, 5:23 PM.

Details

Summary

Today I've faced with a crash in KTorrent.
My system is Kubuntu 18.04 based on Qt 5.9.5
KTorrent settings has TCP and uTP enabled with uTP primary.
The app crashes just after I change a port number in Settings/Network/Port and press Apply button.

gdb doesn't show anything useful:

[Thread debugging using libthread_db enabled]                                                                                                                                                                                    
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".                                                                                                                                                       
0x00007f8a6ec330b4 in __GI___libc_read (fd=18, buf=0x5607c2cc9a78, nbytes=16384) at ../sysdeps/unix/sysv/linux/read.c:27                                                                                                         
27      ../sysdeps/unix/sysv/linux/read.c: No such file or directory.                                                                                                                                                            
(gdb) continue
Continuing.

Thread 5 "utp::UTPServerT" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f8a550ab700 (LWP 28650)]
0x00007f8a6fb1dbe4 in QSocketNotifier::type() const () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
(gdb) quit

stdout seems to have these messages related to the problem:

Warning: QSocketNotifier: Socket notifiers cannot be enabled or disabled from another thread
Warning: QSocketNotifier: Socket notifiers cannot be enabled or disabled from another thread

The crash happens after call to the globals.getUTPServer().changePort(port); in void Core::applySettings()
On libktorrent side its happen shortly after d->sockets.clear(); in bool UTPServer::changePort(bt::Uint16 p). Debugger stops at exec(); in void UTPServerThread::run()

According to my understanding this happens because d->sockets content as well as UTPServer instance were moved to another thread in void UTPServer::start() and UTPServerThread constructor. While globals.getUTPServer().changePort(port); is called in KTorrent main thread.

This problem seems to exist for a long time, and always trigger warnings but didn't crash the app. After I moved to 18.04 with Qt 5.9.5 this has started to crash the app.

The TCP Server seems to be executed in the main thread and doesn't have this problem.

There may be plenty of ways to fix this. The easiest one (but may be not the best one) is to avoid calling changePort(port); for UTPServer and stop/start it with the right port instead. In libktorrent's bool Globals::initUTPServer(Uint16 port) the utp_server->changePort() is called before utp_server->start() spawns a new thread. This is proposed here.

I would like to know if this problem is reproducible on other machines and probably someone would like to make UTPServer::changePort() a thread-safe in libktorrent itself instead of walkaronding in its client app.

Feel free to add relevant revewers for this.

Test Plan

Run, Go to Settings/Network, change port, press Apply button. The app should crash.

Diff Detail

Repository
R473 KTorrent
Lint
Lint Skipped
Unit
Unit Tests Skipped
trufanov requested review of this revision.May 22 2018, 5:23 PM
trufanov created this revision.

Hi, the problem is indeed reproducible on my machine and your workaround works here. The crash is gone.

I've just tried this with version 5.1.2. The app has crashed. So problem is still exists. Perhaps would be better to apply this patch?

stikonas accepted this revision.Nov 2 2019, 11:36 PM
I've just tried this with version 5.1.2. The app has crashed. So problem is still exists. Perhaps would be better to apply this patch?

Yeah, I think apply this patch. I was hoping for maybe libktorrent fix but I think it won't be coming any time soon.

Maybe also open a new bug, so that libktrorrent's bug is not forgotten.

This revision is now accepted and ready to land.Nov 2 2019, 11:36 PM
trufanov closed this revision.Nov 3 2019, 9:51 AM

I thought this review should be autoclosed but it seems i misspelled something in commit's description.
Anyway, the changes are pushed: https://github.com/KDE/ktorrent/commit/4347981f3c2d1f1aa71c338d54807e627dea3950