errors when importing WorkingTree on a thread due to signals

We shouldn't be doing this unconditionally or at load time, only (probably) when a TextUIFactory is constructed.

Patches to change this gratefully accepted.

we'd still want to only do it once, other than that I agree that there is no need to do it on import.

I've got tests running on a patch now.

Branch proposed for merging.

I see that bzr 2.1 final has been released without this bugfix. Developers working on Ubuntu One will need to run a patched version of bzr in order to be able to work on our code (we are all using Lucid), and when we upgrade the production servers to Lucid we will run into this bug there as well.

Could this patch be included in the bzr 2.1 package that gets uploaded to lucid?

Hi Elliot,

We don't use fixcommitted, just inprogress til it's done.

We will put this into 2.1.1 which is likely to get into lucid before the final release. I added a bugtask for that.

Thanks! I only marked the ubuntuone-servers bugtask fix committed.

This also breaks trac-bzr with tracd. I assume tracd must cause plugins to be imported in a different thread.

will be in 2.1.1

will be in 2.2.0b1

This has been fixed in https://edge.launchpad.net/bzr/2.1/2.1.1 which I've just released.

Do you want/need us to request 2.1.1 to go into Lucid (which currently has 2.1.0)? There are some other bugs, but nothing really critical.

Martin Pool wrote:
> This has been fixed in https://edge.launchpad.net/bzr/2.1/2.1.1 which
> I've just released.
>
> Do you want/need us to request 2.1.1 to go into Lucid (which currently
> has 2.1.0)? There are some other bugs, but nothing really critical.

I think the bug preventing 2.1.0's bzrlib being called from other
threads is pretty close to critical and certainly worthy of getting into
Lucid.

Ian C.

On 24 March 2010 18:42, Ian Clatworthy <email address hidden> wrote:
> Martin Pool wrote:
>> This has been fixed in https://edge.launchpad.net/bzr/2.1/2.1.1 which
>> I've just released.
>>
>> Do you want/need us to request 2.1.1 to go into Lucid (which currently
>> has 2.1.0)?  There are some other bugs, but nothing really critical.
>
> I think the bug preventing 2.1.0's bzrlib being called from other
> threads is pretty close to critical and certainly worthy of getting into
> Lucid.

Note that you can call it from other threads, as long as the initial
import is done from the main thread. But that might be hard to
arrange in some apps. So I'll nominate it for Lucid.

--
Martin <http://launchpad.net/~mbp/>

To get into Lucid now, this needs to go through <https://wiki.ubuntu.com/FreezeExceptionProcess#FeatureFreeze+for+bugfix-only+updates>

What follows is the upstream NEWS changes from 2.1.0 (currently in lucid) to 2.1.1:

=== modified file 'NEWS'
--- NEWS 2010-02-16 16:08:40 +0000
+++ NEWS 2010-03-24 08:01:03 +0000
@@ -5,6 +5,66 @@
 .. contents:: List of Releases
    :depth: 1

+
+bzr 2.1.1
+#########
+
+:2.1.1: 2010-03-24
+
+This is a small bugfix release. Upgrading is recommended for anyone
+running 2.1.0 or earlier.
+
+Bug Fixes
+*********
+
+* Allow syscalls to automatically restart when ``TextUIFactory``'s
+ SIGWINCH handler is invoked, avoiding ``EINTR`` errors during blocking
+ IO, which are often poorly handled by Python's libraries and parts of
+ bzrlib. (Andrew Bennetts, #496813)
+
+* Avoid ``malloc(0)`` in ``patiencediff``, which is non-portable.
+ (Martin Pool, #331095)
+
+* Fix plugin packaging on Windows. (Ian Clatworthy, #524162)
+
+* Fix stub sftp test server to call os.getcwdu().
+ (Vincent Ladeuil, #526211, #526353)
+
+* Fixed CHM generation by moving the NEWS section template into
+ a separate file. (Ian Clatworthy, #524184)
+
+* Merge correctly when this_tree is not a WorkingTree. (Aaron Bentley)
+
+* Register SIGWINCH handler only when creating a ``TextUIFactory``; avoids
+ problems importing bzrlib from a non-main thread.
+ (Elliot Murphy, #521989)
+
+* Standardize the error handling when creating a new ``StaticTuple``
+ (problems will raise TypeError). (Matt Nordhoff, #457979)
+
+* Warn if pyrex is too old to compile the new ``SimpleSet`` and
+ ``StaticTuple`` extensions, rather than having the build fail randomly.
+ (John Arbash Meinel, #449776)
+
+Documentation
+*************
+
+* Added a link to the Desktop Guide. (Ian Clatworthy)
+
+* Added What's New in Bazaar 2.1 document. (Ian Clatworthy)
+
+* Drop Google Analytics from the core docs as they caused problems
+ in the CHM files. (Ian Clatworthy, #502010)
+
+API Changes
+***********
+
+* Added ``bzrlib.osutils.set_signal_handler``, a convenience function that
+ can set a signal handler and call ``signal.siginterrupt(signum,
+ False)`` for it, if the platform and Python version supports it.
+ (Andrew Bennetts, #496813)
+
+
 bzr 2.1.0
 #########

@@ -328,23 +388,55 @@
   (Martin Pool)

-bzr 2.0.5 (not released yet)
-############################
+bzr 2.0.5
+#########

 :Codename:
-:2.0.5:
+:2.0.5: NOT RELEASED YET

 Bug Fixes
 *********

+* Avoid ``malloc(0)`` in ``patiencediff``, which is non-portable.
+ (Martin Pool, #331095)
+
+* Concurrent autopacking is more resilient to already-renamed pack files.
+ If we find that a file we are about to obsolete is already obsoleted, we
+ do not try to rename it, and we leave the file in ``obsolete_packs``.
+ The code is also fault tolerant if a file goes missing, assuming that
+ another process already removed the file.
+ (John Arbash Meinel, Gareth White, #507557)
+
+* Cope with the lockdir ``held/info`` file being empty, which seems to
+ happen fairly often if the process is suddenly interrupted while taking
+ a lock.
+ (Martin Pool, #185103)
+
+* Give the warning about potentially slow cross...

Here is the diff for review. It has some bulk updates to the copyright headers but is pretty easily reviewable.

Hi Ubuntu release team, I think this bugfix release is worthy of going into Lucid immediately. This bug breaks Ubuntu One servers, trac-bzr, and I suspect it may break loggerhead as well.

I've briefly reviewed the other bugfixes included in this micro release, and see nothing that should block this from going into Lucid before beta2.

This bug was fixed in the package bzr - 2.2.0-1

---------------
bzr (2.2.0-1) unstable; urgency=low

  * New upstream release.
   + Adds support for setting timestamps to originating revisions.
     Closes: #473450
   + Removes remaining string exception. Closes: #585193, LP: #586926
   + Add C extension to work around Python issue 1628205. LP: #583941,
     Closes: #577110
   + Avoids showing progress bars when --quiet is used. Closes: #542105,
     LP: #320035
   + No longer creates ~/.bazaar as root when run under sudo. LP: #376388
   + 'bzr commit' now supports -p as alternative for --show-diff. LP: #571467
   + 'bzr add' no longer adds .THIS/.BASE/.THEIRS files unless
     explicitly requested. LP: #322767
   + When parsing patch files, Bazaar now supports diff lines before each
     patch. LP: #502076
   + WorkingTrees now no longer requires using signal.signal, so can
     be used in a threaded environment. LP: #521989
   + An assertion error is no longer triggered when pushing to a pre-1.6
     Bazaar server. LP: #528041
  * Bump standards version to 3.9.1.

bzr (2.2.0~b4-1) experimental; urgency=low

  * New upstream beta.
  * Bump standards version to 3.9.0.
  * Drop build dependency on zlib.

bzr (2.2.0~b2-1) experimental; urgency=low

  * New upstream release.
  * Recommend python-launchpadlib. Closes: #568937

bzr (2.1.2-1.1) unstable; urgency=low

  * Non-maintainer upload.
  * Fix access via http_proxy. Closes: #588430
 -- Jelmer Vernooij <email address hidden> Sat, 07 Aug 2010 00:54:52 +0200