[Python-Dev] Help preventing SIGPIPE/SIG_DFL anti-pattern.
Hello,
I'm looking for someone in the python community to help with a problem
of anti-patterns showing up dealing with SIGPIPE.
Specifically I've noticed an anti-pattern developing where folks will
try to suppress broken pipe errors written to stdout by setting
SIGPIPE's disposition to SIG_DFL. This is actually very common, and
also rather broken due to the fact that for all but the most simple text
filters this opens up a problem where the process can exit unexpectedly
due to SIGPIPE being generated from a remote connection the program makes.
I have attached a test program which shows the problem.
to use this program it takes several args.
# 1. Illustrate the 'ugly output to stderr' that folks want to avoid:
% python3 t0.py nocatch | head -1
# 2. Illustrate the anti-pattern, the program exits on about line 47
which most folks to not understand
% python3 t0.py dfl | head -1
# 3. Show a better solution where we catch the pipe error and cleanup to
avoid the message:
% python3 t0.py | head -1
I did a recent audit of a few code bases and saw this pattern pop often
often enough that I am asking if there's a way we can discourage the use
of "signal(SIGPIPE, SIG_DFL)" unless the user really understands what
they are doing.
I do have a pull req here: https://github.com/python/cpython/pull/6773
where I am trying to document this on the signal page, but I can't sort
out how to land this doc change.
thank you,
-Alfred
#
# Program showing the dangers of setting the SIG_PIPE handler to the default handler (SIG_DFL).
#
# To illustrate the problem run with:
# ./foo.py dfl
#
# The program will exit in do_network_stuff() even though there is a an "except" clause.
# The do_network_stuff() simulates a remote connection that closes before it can be written to
# which happens often enough to be a hazard in practice.
#
#
#
import signal
import sys
import socket
import os
def sigpipe_handler(sig, frame):
sys.stderr.write("Got sigpipe \n\n\n")
sys.stderr.flush()
def get_server_connection():
# simulate making a connection to a remote service that closes the connection
# before we can write to it. (In practice a host rebooting, or otherwise exiting while we are
# trying to interact with it will be the true source of such behavior.)
s1, s2 = socket.socketpair()
s2.close()
return s1
def do_network_stuff():
# simulate interacting with a remote service that closes its connection
# before we can write to it. Example: connecting to an http service and
# issuing a GET request, but the remote server is shutting down between
# when our connection finishes the 3-way handshake and when we are able
# to write our "GET /" request to it.
# In theory this function should be resilient to this, however if SIGPIPE is set
# to SIGDFL then this code will cause termination of the program.
if 'dfl' in sys.argv[1:]:
signal.signal(signal.SIGPIPE, signal.SIG_DFL)
for x in range(5):
server_conn = get_server_connection()
sys.stderr.write("about to write to server socket...\n")
try:
server_conn.send(b"GET /")
except BrokenPipeError as bpe:
sys.stderr.write("caught broken pipe on talking to server, retrying...")
def work():
do_network_stuff()
for x in range(1):
print("y")
sys.stdout.flush()
def main():
if 'nocatch' in sys.argv[1:]:
work()
else:
try:
work()
except BrokenPipeError as bpe:
signal.signal(signal.SIGPIPE, signal.SIG_DFL)
os.kill(os.getpid(), signal.SIGPIPE)
if __name__ == '__main__':
main()
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
[Python-Dev] Help preventing SIGPIPE/SIG_DFL anti-pattern.
(sorry for the double post, looks like maybe attachments are dropped,
inlined the attachment this time.)
Hello,
I'm looking for someone in the python community to help with a problem
of anti-patterns showing up dealing with SIGPIPE.
Specifically I've noticed an anti-pattern developing where folks will
try to suppress broken pipe errors written to stdout by setting
SIGPIPE's disposition to SIG_DFL. This is actually very common, and
also rather broken due to the fact that for all but the most simple text
filters this opens up a problem where the process can exit unexpectedly
due to SIGPIPE being generated from a remote connection the program makes.
I have attached a test program which shows the problem.
to use this program it takes several args.
# 1. Illustrate the 'ugly output to stderr' that folks want to avoid:
% python3 t0.py nocatch | head -1
# 2. Illustrate the anti-pattern, the program exits on about line 47
which most folks to not understand
% python3 t0.py dfl | head -1
# 3. Show a better solution where we catch the pipe error and cleanup to
avoid the message:
% python3 t0.py | head -1
I did a recent audit of a few code bases and saw this pattern pop often
often enough that I am asking if there's a way we can discourage the use
of "signal(SIGPIPE, SIG_DFL)" unless the user really understands what
they are doing.
I do have a pull req here: https://github.com/python/cpython/pull/6773
where I am trying to document this on the signal page, but I can't sort
out how to land this doc change.
thank you,
-Alfred
=== CUT HERE ===
#
# Program showing the dangers of setting the SIG_PIPE handler to the
default handler (SIG_DFL).
#
# To illustrate the problem run with:
# ./foo.py dfl
#
# The program will exit in do_network_stuff() even though there is a an
"except" clause.
# The do_network_stuff() simulates a remote connection that closes
before it can be written to
# which happens often enough to be a hazard in practice.
#
#
#
import signal
import sys
import socket
import os
def sigpipe_handler(sig, frame):
sys.stderr.write("Got sigpipe \n\n\n")
sys.stderr.flush()
def get_server_connection():
# simulate making a connection to a remote service that closes the
connection
# before we can write to it. (In practice a host rebooting, or
otherwise exiting while we are
# trying to interact with it will be the true source of such behavior.)
s1, s2 = socket.socketpair()
s2.close()
return s1
def do_network_stuff():
# simulate interacting with a remote service that closes its connection
# before we can write to it. Example: connecting to an http
service and
# issuing a GET request, but the remote server is shutting down between
# when our connection finishes the 3-way handshake and when we are able
# to write our "GET /" request to it.
# In theory this function should be resilient to this, however if
SIGPIPE is set
# to SIGDFL then this code will cause termination of the program.
if 'dfl' in sys.argv[1:]:
signal.signal(signal.SIGPIPE, signal.SIG_DFL)
for x in range(5):
server_conn = get_server_connection()
sys.stderr.write("about to write to server socket...\n")
try:
server_conn.send(b"GET /")
except BrokenPipeError as bpe:
sys.stderr.write("caught broken pipe on talking to server,
retrying...")
def work():
do_network_stuff()
for x in range(1):
print("y")
sys.stdout.flush()
def main():
if 'nocatch' in sys.argv[1:]:
work()
else:
try:
work()
except BrokenPipeError as bpe:
signal.signal(signal.SIGPIPE, signal.SIG_DFL)
os.kill(os.getpid(), signal.SIGPIPE)
if __name__ == '__main__':
main()
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Help preventing SIGPIPE/SIG_DFL anti-pattern.
On 6/30/18 4:20 PM, Greg Ewing wrote: Alfred Perlstein wrote: I am asking if there's a way we can discourage the use of "signal(SIGPIPE, SIG_DFL)" unless the user really understands what they are doing. Maybe there's some way that SIGPIPEs on stdout could be handled differently by default, so that they exit silently instead of producing an ugly message. That would remove the source of pain that's leading people to do this. Thank you Greg, I can poke around into this, it would be a bit of a challenge as the descriptor which causes BrokenPipeError does not appear to be stored within the exception so differentiating it from other exceptions might be a bit tricky. I will look into this in the coming weeks. Any tips on accomplishing this? I was thinking of encoding the fd responsible for causing the error into the exception somehow and then checking to see if it was stdout, then not reporting on it. -Alfred ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
[Python-Dev] review for doctest fix: bpo-35753
Hello, We are using doctest to give our developers easy access to write very fast unit tests and encourage "tests as documentation". Recently we hit an issue where doctest crashes on certain objects that fail to "unwrap". Looking at the code and reasoning behind the crash it seems like we can simply ignore a failed call to unwraps. There is now a PR open against against trunk and 3.8 branch (happy to make a 3.9 branch PR as well) here: trunk: https://github.com/python/cpython/pull/22981 3.8: https://github.com/python/cpython/pull/22834 Does anyone have time to review and/or comment on this? Thank you, -Alfred ___ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-le...@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/HXN27UKPYB7E2MGKL2I32HFS7YKHNGUT/ Code of Conduct: http://python.org/psf/codeofconduct/
[Python-Dev] Re: review for doctest fix: bpo-35753
On 11/6/20 6:13 PM, Steven D'Aprano wrote: For the benefit of others, the problem is that `unittest.mock.call.__wrapped__` generates a new object, which in turn has a dynamic `__wrapped__` attribute, which does the same, thus generating an infinite chain of *distinct* proxies. Being distinct proxy objects defeats the loop detection algorithm of `inspect.unwrap`, which raises ValueError when the recursion limit is reached, and that breaks doctest. (I am explicitly stating that here because I spent an embarassingly long time misunderstanding the nature of the bug, then more time digging into the PR and bug track issues to understand what it was, rather than what it isn't. Maybe I can save anyone else from my misunderstanding.) I'm not convinced that this should be fixed by catching the ValueError inside doctest. Or at least, not *just* by doing so. There's a deeper problem that should be fixed, outside of doctest. Looking at the similar issue here: https://bugs.python.org/issue25532 `mock.call` has broken other functions in the past, and will probably continue to do so in the future. I don't think this infinite chain is intentional, I think it just happens by accident, which makes this a bug in `call`. I think. Michael Foord (creator of mock, if I recall correctly) suggested blacklisting `__wrapped__` from the proxying: https://bugs.python.org/issue25532#msg254726 which I think is the right solution, rather than touching doctest. Michael also said he wasn't happy with an arbitrary limit on the depth of proxies, but I would say that limiting the depth to sys.getrecursionlimit() is not arbitrary and should avoid or at least mitigate the risk of infinite loops and/or memory exhaustion in the general case of arbitrary attribute lookups: py> a = unittest.mock.call py> for i in range(5): # for arbitrary large values of 5 ... a = a.wibble ... py> a wibble.wibble.wibble.wibble.wibble I'm not a mock expert, but I guess such mock dynamic lookups should be limited to the recursion limit. Currently they will loop forever or until you run out of memory. Setting `call.__wrapped__` to None seems to directly fix the problem with doctest: [steve@susan test]$ cat demo2.py """ Run doctest on this module. """ from unittest.mock import call call.__wrapped__ = None [steve@susan test]$ python3.9 -m doctest -v demo2.py 1 items had no tests: demo2 0 tests in 1 items. 0 passed and 0 failed. Test passed. but I don't know if that will break any uses of `mock.call`. Another fix (untested) would be to explicitly test for mocked call: inspect.unwrap(val, stop=lambda obj: isinstance(obj, unittest.mock._Call)) but I don't like that much. I agree. It would be nice to fix mock.call, however that would leave a gap in doctest which it can fail for strange objects that behave similarly for no good reason. Source: https://bugs.python.org/issue25532#msg273272 > Nick Coghlan (ncoghlan) > unittest.mock.Mock is the only case of that in the standard library, but it's far from the only Python mocking library out there, and we should give a clear exception in such cases, rather than eating up all the memory on the machine. Doctest existed for a very long time ~2001 through 2014 without the "vulnerable" (sorry, just calling it that) call to "unwrap" in place, hence the unwrap itself is not really key to doctest working or not. Adding unwrap without the check for unwrap failing is a bug on par with the unwrap problem with mock.call itself. Based on defensive coding and that doctest should not fail in difficult to understand ways I think it makes sense to me to fix both mock.call AND doctest, not only one. -Alfred ___ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-le...@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/T6ZS2YMOIAJIZCU443MQPEC2UHNY26W6/ Code of Conduct: http://python.org/psf/codeofconduct/
[Python-Dev] Re: Ideas for improving the contribution experience
On 10/16/20 3:29 PM, Tal Einat wrote:
(Context: Continuing to prepare for the core dev sprint next week.
Since the sprint is near, *I'd greatly appreciate any quick comments,
feedback and ideas!*)
Following up my collection of past beginning contributor experiences,
I've collected these experiences in a dedicated GitHub repo[1] and
written a (subjective!) summary of main themes that I recognize in the
stories, which I've also included in the repo[2].
A "TL;DR" bullet list of those main themes:
* Slow/no responsiveness
* Long, slow process
* Hard to find where to contribute
* Mentorship helps a lot, but is scarce
* A lot to learn to get started
* It's intimidating
More specifically, something that has come up often is that
maintaining momentum for new contributors is crucial for them to
become long-term contributors. Most often, this comes up in relation
to the first two points: Suggestions or PRs are completely receive no
attention at all ("ignored") or stop receiving attention at some point
("lost to the void"). Unfortunately, the probability of this is pretty
high for any issue/PR, so for a new contributor this is almost
guaranteed to happen while working on one of their first few
contributions. I've seen this happen many times, and have found that I
have to personally follow promising contributors' work to ensure that
this doesn't happen to them. I've also seen contributors learn to
actively seek out core devs when these situations arise, which is
often a successful tactic, but shouldn't be necessary so often.
Now, this is in large part a result of the fact that us core devs are
not a very large group, made up almost entirely of volunteers working
on this in their spare time. Last I checked, the total amount of paid
development time dedicated to developing Python is less than 3
full-time (i.e. ~100 hours a week).
The situation being problematic is clear enough that the PSF had
concrete plans to hire paid developers to review issues and PRs.
However, those plans have been put on hold indefinitely, since the
PSF's funding has shrunk dramatically since the COVID-19 outbreak (no
PyCon!).
So, what can be done? Besides raising more funds (see a note on this
below), I think we can find ways to reduce how often issues/PRs become
"stalled". Here are some ideas:
1. *Generate reminders for reviewers when an issue or PR becomes
"stalled' due to them.* Personally, I've found that both b.p.o. and
GitHub make it relatively hard to remember to follow up on all of the
many issues/PRs you've taken part in reviewing. It takes considerable
attention and discipline to do so consistently, and reminders like
these would have helped me. Many (many!) times, all it took to get an
issue/PR moving forward (or closed) was a simple "ping?" comment.
2. *Generate reminders for contributors when an issue or PR becomes
"stalled" due to them.* Similar to the above, but I consider these
separate.
3. *Advertise something like a "2-for-1" standing offer for reviews.*
This would give contributors an "official", acceptable way to get
attention for their issue/PR, other than "begging" for attention on a
mailing list. There are good ways for new contributors to be of
significant help despite being new to the project, such as checking
whether old bugs are still relevant, searching for duplicate issues,
or applying old patches to the current code and creating a PR. (This
would be similar to Martin v. Löwis's 5-for-1 offer in 2012[3], which
had little success but lead to some interesting followup discussion[4]).
4. *Encourage core devs to dedicate some of their time to working
through issues/PRs which are "ignored" or "stalled".* This would
require first generating reliable lists of issues/PRs in such states.
This could be in various forms, such as predefined GitHub/b.p.o.
queries, a dedicated web-page, a periodic message similar to b.p.o.'s
"weekly summary" email, or dedicated tags/labels for issues/PRs.
(Perhaps prioritize "stalled" over "ignored".)
- Tal Einat
Tal, this would be great.
Is it possible to get a ballpark idea on how long a PR should/would take
to be merged so that contributors know if they should ping or not?
-Alfred
___
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at
https://mail.python.org/archives/list/python-dev@python.org/message/LPCCKXDAOY2HMQMMLJDYC6PLUX2Y2APQ/
Code of Conduct: http://python.org/psf/codeofconduct/
[Python-Dev] minor PR 22981 waiting review ~20 days: https://github.com/python/cpython/pull/22981
PR 22981 is a minor update to doctest to allow it to safely consume modules which contain objects that cause inspect.unwrap to throw. I believe the review comments in the PR have been addressed at this point for ~20 days. The patch is relatively small, reviews are done, but it is unmerged. Is there a way to "bump" the request for review? thank you, -Alfred ___ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-le...@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/2YFKPBDFX3Q53PMDNWAV3ZKCSTSVZJXJ/ Code of Conduct: http://python.org/psf/codeofconduct/
[Python-Dev] Doctest does not work with mock.call PR #22981
Hello, There's been a bug open where doctest can break if there are proxy objects that fail to unwrap (https://bugs.python.org/issue35753) since python 3.7, this includes when importing 'call' from the 'mock' module. Does someone have time to review PR 22981 (https://github.com/python/cpython/pull/22981). Last feedback from folks with merge capabilities seems to be from November 8th. I've replied a few times in December on the PR but there is no response. -Alfred ___ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-le...@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/DTT4RRYTLQ4WDUJVD673PLHGJBOO3Y4R/ Code of Conduct: http://python.org/psf/codeofconduct/
[Python-Dev] Doctest crashes on unittest.call import. bpo-35753
Hello, I am trying to sort out what can be done for bpo-35753 / https://github.com/python/cpython/pull/22981 Basically any module that does "from unittest.mock import call" will crash the doctest scanner. At this point I'm not sure how to proceed at this point, there is some different ideas on how to fix it, I guess I could try to fix this differently, however I am a little worried that the different fix suggested in the PR will be higher risk, that said if folks would be up for reviewing this change I would give it a shot to code it up. Can't tell right now if there's no interest in reviewing this patch because there is a feeling that it should be done differently, or just lack of interest in doctest. Can I get clued in on what is the best way forward? I am happy to redo patch, but unsure there is enough interest in this problem for an alternative patch to be reviewed. thanks, -Alfred Forwarded Message Subject: [Python-Dev] minor PR 22981 waiting review ~20 days: https://github.com/python/cpython/pull/22981 Date: Mon, 14 Dec 2020 13:25:36 -0800 From: Alfred Perlstein Organization: FreeBSD To: Python Dev PR 22981 is a minor update to doctest to allow it to safely consume modules which contain objects that cause inspect.unwrap to throw. I believe the review comments in the PR have been addressed at this point for ~20 days. The patch is relatively small, reviews are done, but it is unmerged. Is there a way to "bump" the request for review? thank you, -Alfred ___ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-le...@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/2YFKPBDFX3Q53PMDNWAV3ZKCSTSVZJXJ/ Code of Conduct: http://python.org/psf/codeofconduct/ ___ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-le...@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/C24MPE5NUGMIDLLABR7AUONWAZOI5EWH/ Code of Conduct: http://python.org/psf/codeofconduct/
[Python-Dev] Merge request reminder.
Hello, a while back we hit a bug with doctest and mock.call that confused a few people on our team for a while. I dug into fixing it and made doctest more defensive around un-unwrappable calls. I submitted a PR (https://github.com/python/cpython/pull/22981) for an Issue (https://bugs.python.org/issue35753) back in October 25, 2020 which got some review, and then have not heard for several months. I am wondering if it can be reviewed and feedback given on a way forward. Thank you, -Alfred ___ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-le...@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/DQTVEJGORWXPBLXG2P5S6YZBLCEL2DG3/ Code of Conduct: http://python.org/psf/codeofconduct/
