group download .cancel() leaves connection open

Bug #1229463 reported by Barry Warsaw
6
This bug affects 1 person
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

Revision history for this message
Manuel de la Peña (mandel) wrote :

Looking at the cancelation code of the singled download object (since a group download is a simple collection of those) we have the following:

        Q_Q(SingleDownload);
        qDebug() << __PRETTY_FUNCTION__ << _url;

        if (_reply != NULL) {
            // disconnect so that we do not get useless signals
            // and remove the reply
            disconnectFromReplySignals();
            _reply->abort();
            _reply->deleteLater();
            _reply = NULL;
        }

        // remove current data and metadata
        cleanUpCurrentData();
        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.

Revision history for this message
Barry Warsaw (barry) wrote : Re: [Bug 1229463] Re: group download .cancel() leaves connection open

I'm quite convinced that it's the u-d-m that's keeping connections open to the
server and causing the slow s-i shutdowns. I run htop while running my test
suite, filter on `ubuntu-download-manager` and I can see the 3-4 processes for
u-d-m (I'm presuming they're actually threads).

If while in the s-i shutdown code, I list the open files for each of the
processes, I can see IPv4 type open fds which are the connections to the
server. I can't close those from htop, but if I actually kill the process
(and let dbus activation restart it later), then the s-i shutdown code returns
immediately.

Changed in ubuntu-download-manager:
status: New → In Progress
importance: Undecided → High
assignee: nobody → Manuel de la Peña (mandel)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

Fix committed into lp:ubuntu-download-manager at revision 144, scheduled for release in ubuntu-download-manager, milestone 0.3

Changed in ubuntu-download-manager:
status: In Progress → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntu-download-manager - 0.2+13.10.20130928.2-0ubuntu1

---------------
ubuntu-download-manager (0.2+13.10.20130928.2-0ubuntu1) saucy; urgency=low

  [ Manuel de la Pena ]
  * Add BaseTestCase that removes the qDebug output so that we have a
    cleaner tests results output. (LP: #1230210)
  * Allow the hash method to be empty. (LP: #1232050)
  * Use syslog when running as a system bus service rather than logging
    to a file. (LP: #1230236)
  * Remove the XDG implementation and use the standard paths class found
    in qt. (LP: #1226998)
  * Keep track of the replies so that when the daemon is stoppable we
    clear the cache. (LP: #1229463)

  [ Ubuntu daily release ]
  * Automatic snapshot from revision 144
 -- Ubuntu daily release <email address hidden> Sat, 28 Sep 2013 21:33:00 +0000

Changed in ubuntu-download-manager (Ubuntu):
status: New → Fix Released
Changed in ubuntu-download-manager:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.