group download .cancel() leaves connection open
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
ubuntu-download-manager |
Fix Released
|
High
|
Manuel de la Peña | ||
ubuntu-download-manager (Ubuntu) |
Fix Released
|
Undecided
|
Unassigned |
Bug Description
I think I've found the cause of my 134+ second shutdown problems.
Let's say I have two tests. The first tries to download a bunch of files, and cancels the download part of the way through. The second test tries to download some files. In between these two tests, the d/l dbus service is *not* exited, because dbus activation is slow, but we *do* kill the http/https server in between the tests.
I think that if the d/l service gets a cancel, it still keeps a connection open to the server, thus preventing it from shutting down until some timeout occurs. I'm guessing maybe the d/l service has a timeout in the order of 120 seconds or so?
I also think that this scenario prevents the .exit() call from succeeding immediately, but I'm not positive about that yet. Does .exit() always exit the process immediately and clean up after itself?
There may be other situations where the connection to the server is kept open after it should have been closed.
I have to see if I can work around this in my test suite by dropping all client connections, but it would be best to fix this in the d/l service. Maybe it's not a blocker for landing.
Related branches
- Alejandro J. Cura (community): Approve
- PS Jenkins bot: Approve (continuous-integration)
- Barry Warsaw (community): Approve
-
Diff: 203 lines (+86/-11)4 files modifiedlibubuntudownloadmanager/download_manager.cpp (+1/-1)
libubuntudownloadmanager/libubuntudownloadmanager.pro (+5/-5)
libubuntudownloadmanager/request_factory.cpp (+74/-4)
libubuntudownloadmanager/request_factory.h (+6/-1)
Changed in ubuntu-download-manager: | |
status: | New → In Progress |
importance: | Undecided → High |
assignee: | nobody → Manuel de la Peña (mandel) |
Changed in ubuntu-download-manager: | |
status: | Fix Committed → Fix Released |
Looking at the cancelation code of the singled download object (since a group download is a simple collection of those) we have the following:
qDebug() << __PRETTY_FUNCTION__ << _url;
if (_reply != NULL) {
disconnect FromReplySignal s();
_reply- >abort( );
_reply- >deleteLater( );
// disconnect so that we do not get useless signals
// and remove the reply
_reply = NULL;
}
// remove current data and metadata
cleanUpCurrent Data();
emit q->canceled(true);
As soon as the cancel is called if the reply is not null we will call deleteLater, the only diff between a delete and a deleteLater is that deleteLater will delete the object once all the signals of the reply object have been processed.. I could try and use a delete and ignore those signals since we have aborted the request to see if that speeds up the process.