[issue30985] Set closing variable in asyncore at close

2018-09-06 Thread STINNER Victor
STINNER Victor added the comment: I reject the issue. It doesn't seem possible to set the closing attribute without breaking backward compatibility. See discussion: https://github.com/python/cpython/pull/2804 -- resolution: -> rejected stage: -> resolved status: open -> closed

[issue30985] Set closing variable in asyncore at close

2017-07-25 Thread Giampaolo Rodola'
Giampaolo Rodola' added the comment: I would probably feel safer to use "__closed" for all python versions and adopt the same naming convention for any attribute we may want to add in the future. Kinda weird, but asyncore is probably the only case of deprecated module with bad API design

[issue30985] Set closing variable in asyncore at close

2017-07-25 Thread STINNER Victor
STINNER Victor added the comment: It seems like the closing attribute is not *required* to fix bpo-30931, asyncore race condition: http://bugs.python.org/issue30931#msg299162 Another approach doesn't rely on close() "implementation", but more on close() "expected behaviour" (remove the

[issue30985] Set closing variable in asyncore at close

2017-07-25 Thread STINNER Victor
STINNER Victor added the comment: "Giampaolo, people using only 3.7 should probably use asyncio. Fixing asyncore is more important to those that can use only 2.7 (e.g.Centos/RHEL) or have to support both python 3 and 2." IMHO starting to use closing in asyncore *is* a backward incompatible

[issue30985] Set closing variable in asyncore at close

2017-07-25 Thread Guido van Rossum
Changes by Guido van Rossum : -- nosy: -gvanrossum ___ Python tracker ___ ___

[issue30985] Set closing variable in asyncore at close

2017-07-25 Thread Nir Soffer
Nir Soffer added the comment: Giampaolo, people using only 3.7 should probably use asyncio. Fixing asyncore is more important to those that can use only 2.7 (e.g.Centos/RHEL) or have to support both python 3 and 2. Do you think using _closed is safer for backport? This can also clash with

[issue30985] Set closing variable in asyncore at close

2017-07-25 Thread Giampaolo Rodola'
Giampaolo Rodola' added the comment: On the other hand, due to the poor asyncore API, I think it's safer if we land this in 3.7 only. "closing" attribute was never documented so if there's code out there setting it to True that'll crash their app pretty quickly as sockets won't close.

[issue30985] Set closing variable in asyncore at close

2017-07-25 Thread Giampaolo Rodola'
Giampaolo Rodola' added the comment: I welcome this change as it avoids tricks like this one: https://github.com/giampaolo/pyftpdlib/blob/1268bb185cd63c657d78bc33309041628e62360a/pyftpdlib/handlers.py#L537 Any app using asyncore for anything other than an echo server eventually ends up doing

[issue30985] Set closing variable in asyncore at close

2017-07-25 Thread Nir Soffer
Nir Soffer added the comment: The "new" closing attribute is old as asyncore, it was just unused :-) -- ___ Python tracker ___

[issue30985] Set closing variable in asyncore at close

2017-07-24 Thread STINNER Victor
STINNER Victor added the comment: "Fixing closing attribute will allow fixing the races during asyncore.poll() and asyncore.poll2(), see https://github.com/python/cpython/pull/2764; Aha, *slowly* I understand that asyncore is vulnerable to multiple race conditions, and it seems lile we

[issue30985] Set closing variable in asyncore at close

2017-07-24 Thread Nir Soffer
Nir Soffer added the comment: I agree that this change alone is may not be important enough to fix in asyncore today - but this enables easy detection of closed sockets needed for https://github.com/python/cpython/pull/2764 or the alternative https://github.com/python/cpython/pull/2854.

[issue30985] Set closing variable in asyncore at close

2017-07-24 Thread STINNER Victor
STINNER Victor added the comment: Proposed change looks more like an enhancement like a bug fix. I consider that features are not welcome anyone in the asyncore module, since it's now deprecated, replaced with selectors and asyncio. I would prefer to reject this issue. Giampaolo, Serhiy,

[issue30985] Set closing variable in asyncore at close

2017-07-21 Thread Nir Soffer
Changes by Nir Soffer : -- nosy: +haypo ___ Python tracker ___ ___ Python-bugs-list

[issue30985] Set closing variable in asyncore at close

2017-07-21 Thread Nir Soffer
New submission from Nir Soffer: This is an old issue with asyncore - asyncore has a "closing" attribute, but it was never used. Every user has to implement the closing once logic in dispatcher subclasses. Here is a typical fixes in user code: -

[issue30985] Set closing variable in asyncore at close

2017-07-21 Thread Jaume
Changes by Jaume : -- pull_requests: +2854 ___ Python tracker ___ ___

[issue30985] Set closing variable in asyncore at close

2017-07-21 Thread Jaume
Changes by Jaume : -- nosy: walkhour priority: normal severity: normal status: open title: Set closing variable in asyncore at close ___ Python tracker