2.7.8 backport of Issue1856 (avoid daemon thread problems at shutdown) breaks ceph

[ned-deily]

BPO	21963
Nosy	@doko42, @pitrou, @vstinner, @benjaminp

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2014-11-21.01:06:32.064>
created_at = <Date 2014-07-11.21:06:05.854>
labels = ['type-bug', 'release-blocker']
title = '2.7.8 backport of Issue1856 (avoid daemon thread problems at shutdown) breaks ceph'
updated_at = <Date 2014-12-03.00:55:16.514>
user = 'https://github.com/ned-deily'

bugs.python.org fields:

activity = <Date 2014-12-03.00:55:16.514>
actor = 'dmick'
assignee = 'none'
closed = True
closed_date = <Date 2014-11-21.01:06:32.064>
closer = 'pitrou'
components = []
creation = <Date 2014-07-11.21:06:05.854>
creator = 'ned.deily'
dependencies = []
files = []
hgrepos = []
issue_num = 21963
keywords = []
message_count = 17.0
messages = ['222795', '222800', '222801', '227994', '227995', '227996', '228022', '228042', '228056', '228062', '231408', '231409', '231416', '231459', '231461', '231974', '232066']
nosy_count = 8.0
nosy_names = ['doko', 'pitrou', 'vstinner', 'benjamin.peterson', 'neologix', 'python-dev', 'Joe.Julian', 'dmick']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue21963'
versions = ['Python 2.7']

[ned-deily]

doko in msg222768 of bpo-1856:

http://tracker.ceph.com/issues/8797 reports that the backport to 2.7 causes a regression in ceph.

[vstinner]

Does anyone have a *simple* script to reproduce the regression?

The changeset 7741d0dd66ca looks good to me, Python 3 does the same thing since Python 3.2 (the new GIL).

Anyway, daemon threads are evil :-( Expecting them to exit cleanly automatically is not good. Last time I tried to improve code to cleanup Python at exit in Python 3.4, I also had a regression (just before the release of Python 3.4.0): see the issue bpo-21788.

[vstinner]

The changeset 7741d0dd66ca looks good to me, Python 3 does the same thing since Python 3.2 (the new GIL).

Oh, I forgot to say that the simplest way to fix the regression is to revert this commit... As I wrote modifying the code to cleanup Python at exit is risky, and it's maybe too late to do that in Python 2.7?

The risk of regression caused by 7741d0dd66ca is maybe higher than the benefit of the enhancement? :-(

[pitrou]

I would favour reverting. The crashiness of daemon threads is not new, so fixing it late in the 2.7 cycle wasn't really necessary IMO.

[pitrou]

As a sidenote, I wonder if the Ceph issue would also occur on 3.x. I guess we won't know until Ceph is ported (is it?).

[pitrou]

Note that this may very well be a bug in Ceph... Is any of the Ceph authors nosied here?

[vstinner]

@benjamin: What do you think? Should we revert the changeset 7741d0dd66ca in Python 2.7?

[benjaminp]

Antoine is probably right that we should revert. I wonder what the exact
problem with Ceph is, though.

On Tue, Sep 30, 2014, at 18:55, STINNER Victor wrote:

STINNER Victor added the comment:

@benjamin: What do you think? Should we revert the changeset 7741d0dd66ca
in Python 2.7?

----------

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue21963\>

---

[neologix]

Agreed with Antoine and Benjamin.

[vstinner]

Oh by the way, I also prefer to revert the commit.

[dmick]

Hi; I'm the original author of the code in the Ceph CLI.

The reason it does what it does is that the Python CLI calls into librados (Ceph, through ctypes) to connect to the cluster; that connection can block for various reasons, so it's spawned in a thread; after a timeout or ^C, we desire to exit, but if we've got a non-daemon thread stuck in Ceph, and no way to cancel it from the threading module, sys.exit() will also block waiting for the Ceph thread to finally give up. If, however, we set that thread as a daemon thread, it doesn't block the sys.exit() (that being, I thought, the whole point of daemon threads).

I confess I don't fully understand the change in 7741d0dd66ca, but it does seem to have the side effect of not actually allowing exit while there are outstanding daemon threads not hitting Python.

[JoeJulian]

I suspect the underlying problem is that the fix expects the daemon threads to hit a point where they try to acquire the GIL and that's not going to happen with these librados threads; they stay in librados.

[pitrou]

Thanks for the explanation.

I confess I don't fully understand the change in 7741d0dd66ca, but it
does seem to have the side effect of not actually allowing exit while
there are outstanding daemon threads not hitting Python.

That's a bit weird, as that change doesn't introduce any wait. Are you sure there isn't something else happening, e.g. a lock is owned by the daemon thread and a non-daemon thread tries to acquire it?

[python-dev]

New changeset 4ceca79d1c63 by Antoine Pitrou in branch '2.7':
Issue bpo-21963: backout issue bpo-1856 patch (avoid crashes and lockups when
https://hg.python.org/cpython/rev/4ceca79d1c63