Ubuntu
bzr package

[SRU] dput sftp upload hangs after all files successfully uploaded

Bug #659590 reported by Max Bowsher
26
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Bazaar
Fix Released
High
Max Bowsher
Bazaar 2.2.2
2.3
Fix Released
Critical
Max Bowsher
Bazaar 2.3b4
bzr (Ubuntu)
Fix Released
Undecided
Max Bowsher
Lucid
Won't Fix
Undecided
Unassigned
Maverick
Fix Released
Undecided
Unassigned

Bug Description

Nature of problem:

I am experiencing sftp uploads to ppa.launchpad.net reliably failing to terminate. The dput process appears to be waiting for the ssh (sftp) subprocess to terminate, but it never does. When killed, the upload has actually been successful, and is accepted by Launchpad.

Reason for SRUing:

Launchpad is trying to promote SFTP uploads as the better way to upload packages, so that it can know who did an upload even if the upload is unsigned or wrongly signed, but SFTP uploads currently do not work.

How addressed:

Leakage of subprocess stdout/stdin filehandles, which caused the subprocess to never exit, which caused the parent process to wait forever for it to do so, has been fixed.

Minimal patch:

Please see diff in this MP: https://code.launchpad.net/~maxb/bzr/2.2-close-ssh-subprocess-socket/+merge/40493

TEST CASE:
Attempt to dput any source package to ppa.launchpad.net via SFTP. If the dput process exits successfully, this is a success. If the bug is present, the dput process hangs waiting for a child ssh process to exit, but it never does.

To do this:

1) Configure dput to use sftp. In ~/.dput.cf, add:
[ppa]
fqdn = ppa.launchpad.net
method = sftp
incoming = ~%(ppa)s/ubuntu
login = your-lp-id

2) Get a debian source package, it doesn't matter what. e.g. use `apt-get source hello`. Build it, so you have a valid signed *_source.changes file.

3) dput ppa:not-a-valid-ppa-we-just-want-to-see-if-uploading-works hello_2.5-1_source.changes

Regression potential: Very slim, it's a shallow and well understood bug, with small diff to fix.

MicroReleaseException: bzr has a MRE, so rather than a targetted backport, an upstream micro release will be used, additionally providing fixes for:
   + Don't crash when --take-other or --take-this is called for a text
     conflict. LP: #646961
   + Make 'bzr commit' in a checkout propagate new tags to the master.
     LP: #603395

I will need a sponsored upload. I hope to have this occur by a sponsor using 'bzr builddeb' from lp:~maxb/ubuntu/maverick/bzr/sru - I will provide a .dsc and .debian.tar.gz if that's not acceptable.

-------- STOP PRESS --------

This SRU may not happen like this. I've just discovered our MicroReleaseException is broken. We cannot run the testsuite during package build, because bzr is in main, but python-testtools is in universe.

UPDATE: This has been resolved, see comment #5.

See original description
Revision history for this message
Max Bowsher (maxb) wrote :
Revision history for this message
Max Bowsher (maxb) wrote :

It seems this is due to a regression in bzrlib, such that it does not close the stdin/out to the child process.

Vincent Ladeuil (vila)
Changed in bzr:
milestone: none → 2.2.2
assignee: nobody → Max Bowsher (maxb)
importance: Undecided → High
status: New → Fix Released
Revision history for this message
Max Bowsher (maxb) wrote :

Retarget to bzr source package.

affects: dput (Ubuntu) → bzr (Ubuntu)
Changed in bzr (Ubuntu):
assignee: nobody → Max Bowsher (maxb)
Max Bowsher (maxb)
summary: - sftp upload hangs after all files successfully uploaded
+ dput sftp upload hangs after all files successfully uploaded
Max Bowsher (maxb)
description: updated
Revision history for this message
Vincent Ladeuil (vila) wrote : Re: dput sftp upload hangs after all files successfully uploaded

Marking as critical (even if the fix has already landed on trunk) so we don't forget to use this bug as our SRU flag for 2.2.2 once 2.3b4 is released.

tags: added: sru
Revision history for this message
Martin Pool (mbp) wrote :

The technical board resolved[1][2] in December that they would allow SRUs of Bazaar 2.2 without running the test suite during the build, because the dependency problem that prevents it is not feasible to fix in Maverick, and because it can be adequately tested at other steps of the process.

As part of the SRU process we should run 'bzr selftest' after installing the package into a clean system, and that should pass with no errors in bzr 2.2.2.

We should now proceed with a SRU into Maverick.

[1] https://lists.ubuntu.com/archives/technical-board/2010-December/000632.html
[2] https://wiki.ubuntu.com/StableReleaseUpdates/MicroReleaseExceptions

summary: - dput sftp upload hangs after all files successfully uploaded
+ [SRU] dput sftp upload hangs after all files successfully uploaded
description: updated
Revision history for this message
Martin Pool (mbp) wrote :

Dear Max / ubuntu-sru team:

bzr-2.2.2, including this fix, should be included in maverick-updates. I don't know of any blockers at this point.

I apparently can't nominate this for a release in Launchpad, and I can't upload it to -proposed. Could you please do these steps for me:

> Use Nominate for release to mark the bug as an SRU candidate for the appropriate Ubuntu releases (e. g. the current LTS and latest stable release), then subscribe ubuntu-sru.
> Upload the fixed package to release-proposed with the patch in the bug report, a detailed and user-readable changelog, and no other unrelated changes. Make sure that the version number does not conflict with any later and future version in other Ubuntu releases (the security policy document has a well-working scheme which can be used for SRUs.) Also be sure to reference the SRU bug number in the changelog using the 'LP: #NNNNNN' convention. If you need a sponsored upload, attach a debdiff to the bug and subscribe ubuntu-sponsors, as usual. There is no need to wait before uploading .

Martin Pool (mbp)
Changed in bzr (Ubuntu):
status: New → In Progress
Brian Murray (brian-murray)
Changed in bzr (Ubuntu Lucid):
status: New → Triaged
Changed in bzr (Ubuntu Maverick):
status: New → Triaged
Jelmer Vernooij (jelmer)
Changed in bzr (Ubuntu):
status: In Progress → Fix Released
Revision history for this message
Colin Watson (cjwatson) wrote : Please test proposed package

Accepted bzr into maverick-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Changed in bzr (Ubuntu Maverick):
status: Triaged → Fix Committed
tags: added: verification-needed
Revision history for this message
Martin Pool (mbp) wrote :

Verification:

I installed bzr 2.2.4-0ubuntu1 into a maverick chroot and tried Max's reproduction instructions. It fails to ask for a password:

Uploading to ppa (via sftp to ppa.launchpad.net):
  bzr_2.2.4-0ubuntu1.dsc: No handlers could be found for logger "bzr"

<bound method SilentUIFactory.get_password of <bzrlib.ui.SilentUIFactory object at 0x14564d0>>

This is probably fixed upstream, has to do with how dput is calling bzr, and is not a regression. I installed openssh-client, which is probably what people want to use rather than paramiko, and ran an ssh-agent. The upload succeeded.

verified ok.

tags: added: verification-ok
removed: verification-needed
tags: added: verification-done
removed: verification-ok
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package bzr - 2.2.4-0ubuntu1

---------------
bzr (2.2.4-0ubuntu1) maverick-proposed; urgency=low

  [ Jelmer Vernooij ]
  * Update watch file to use 2.2 series.
  * New upstream release.
   + Fixes closing of leaked sockets to SSH subprocesses, which causes
     dput sftp uploads to hang. LP: #659590
   + Fixes the use of 'lp:' urls behind a http proxy. LP: #558343
   + Correctly sets the Content-Type header when http POSTing to comply
     with stricter web frameworks. LP: #665100
   + Fixes propagating tags to the master branch in a bound branch or
     heavyweight checkout. LP: #603395
   + Fixes the use of 'bzr resolve --take-other' if the file is
     involved in an unresolved text conflict. LP: #646961
   + Fixes https access with newer versions of python2.7. LP: #693880
   + Fixes crash during pack caused by a concurrent repository pack
     operation. LP: #701940
   + Fixes communication with the Launchpad web service when using
     launchpadlib >= 1.5.5. LP: #707075
   + Switches away from deprecated 'edge.launchpad.net' LP: #583667
   + Fixes resolving of content (and path) conflicts for files in subdirs.
     LP: #660935
   + Fixes nasty recursion loop while displaying branch opening error.
     LP: #687653

  [ Martin Pool ]
  * Propose for maverick SRU.
 -- Martin Pool <email address hidden> Thu, 07 Apr 2011 15:30:17 +1000

Changed in bzr (Ubuntu Maverick):
status: Fix Committed → Fix Released
Ubuntu Foundations Team Bug Bot (crichton)
tags: added: testcase
Revision history for this message
Rolf Leggewie (r0lf) wrote :

lucid has seen the end of its life and is no longer receiving any updates. Marking the lucid task for this ticket as "Won't Fix".

Changed in bzr (Ubuntu Lucid):
status: Triaged → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

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