Implementation of the PEP 524
Hi, I wrote a first implementation of the PEP 524 (make os.urandom blocking): * https://bugs.python.org/issue27776 : "PEP 524: Make os.urandom() blocking on Linux" * https://bugs.python.org/issue27778 : "PEP 524: Add os.getrandom()" My patches are now waiting for your review :-) You should also review changes made to prepare the issue #27776: https://hg.python.org/cpython/rev/980e2c781810 https://hg.python.org/cpython/rev/265644bad99e https://hg.python.org/cpython/rev/86d0d74bc2e1 https://hg.python.org/cpython/rev/ad141164c792 The last change (close fd on error) is not a really bugfix, it's more a new feature. In Python 3.5, it wasn't needed to close the file descriptor since Py_FatalError() was called immediatly. Victor
On 17 August 2016 at 03:28, Victor Stinner
Hi,
I wrote a first implementation of the PEP 524 (make os.urandom blocking):
* https://bugs.python.org/issue27776 : "PEP 524: Make os.urandom() blocking on Linux" * https://bugs.python.org/issue27778 : "PEP 524: Add os.getrandom()"
My patches are now waiting for your review :-)
Thanks for tackling this, Victor! I'll do a proper review tomorrow (OK, technically, later today), but could we take a slightly different approach to handling the new "blocking" parameter in py_getrandom? Specifically, I'd like to still make the initial call with GRND_NONBLOCK, then have conditional handling of EAGAIN such that: - if blocking is not set, it behaves as it does now - if blocking *is* set, it prints a warning to stderr, before trying again without the GRND_NONBLOCK flag That would address the main problem I was worried about in PEP 522, which is folks potentially being faced with an unexpected application or service hang, and few clues about where to start in debugging it. The only downside I see is needing two syscalls instead of one in the blocking case, but I'd expect the "blocking" part to be the main delay there, rather than the second syscall. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
2016-08-17 18:39 GMT+02:00 Nick Coghlan
- if blocking *is* set, it prints a warning to stderr, before trying again without the GRND_NONBLOCK flag
I implemented the PEP 524. Such warning is not part of the PEP. Sorry, but I'm not interested to reopen the discussion on the PEP since the PEP was accepted... Victor
On 18 August 2016 at 03:16, Victor Stinner
2016-08-17 18:39 GMT+02:00 Nick Coghlan
: - if blocking *is* set, it prints a warning to stderr, before trying again without the GRND_NONBLOCK flag
I implemented the PEP 524. Such warning is not part of the PEP.
Sorry, but I'm not interested to reopen the discussion on the PEP since the PEP was accepted...
OK, I'll review it on that basis, and then submit a follow-up RFE and patch to add the warning. I'd like to minimise the divergence between upstream and Fedora, and we'll at least have the warning in the downstream version. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 18 August 2016 at 22:10, Nick Coghlan
On 18 August 2016 at 03:16, Victor Stinner
wrote: 2016-08-17 18:39 GMT+02:00 Nick Coghlan
: - if blocking *is* set, it prints a warning to stderr, before trying again without the GRND_NONBLOCK flag
I implemented the PEP 524. Such warning is not part of the PEP.
Sorry, but I'm not interested to reopen the discussion on the PEP since the PEP was accepted...
OK, I'll review it on that basis, and then submit a follow-up RFE and patch to add the warning.
Mostly +1 from me on the code and test changes in both patches (I did have a few requests for clarification and confirmation, but nothing major). Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 18 August 2016 at 22:10, Nick Coghlan
OK, I'll review it on that basis, and then submit a follow-up RFE and patch to add the warning.
I'd like to minimise the divergence between upstream and Fedora, and we'll at least have the warning in the downstream version.
It belatedly occurs to me that I should mention a relevant piece of my work history that may help explain why I'm so strongly in favour of providing some kind of visible notification as to why the Python process is blocked waiting for the kernel (and am happy to tackle it myself as a follow-up patch post-PEP implementation): I spent a couple of years as the development lead for Red Hat's main hardware integration testing system, https://beaker-project.org/ While thankfully rare, the single most difficult operating system level regressions for people to debug in that environment are those where: - the system hangs on startup - before the sshd daemon is running - with no relevant messages on the system console log The cases where guest recipes do this aren't *as* bad (since you can still ssh into the VM host and poke around in the guest via the hypervisor tooling), but when it happens on bare metal, you either need to have physical access to the relevant hardware lab yourself, or get help from a colleague who does. For the os.urandom change, the first two points on that list are entirely out of CPython's control (since they depend on what, if anything, is calling os.urandom() early in the Linux boot cycle), but we *can* influence the third one (by emitting a warning on stderr before we block). Adding such a warning via a downstream patch in Fedora would mostly be sufficient to address the risk for the users of Fedora's & Red Hat's Beaker installations (aside from a few situations involving booting other Linux distros as VM guests), since the two cases where I expect we may see problems are those I mentioned in PEP 522: - testing on ARM hardware that doesn't have a proper entropy source set up - regression tests that run VMs without configuring a proper entropy source for the VM and seeing "<module>:<line>: RuntimeWarning: Waiting for system RNG" in the system console log should be enough to put people on the right path to identifying the root cause of the problem. However, other Linux distros tend to have similar regression testing systems (e.g. it was the Debian one that picked up the issue with 3.5.0), so it makes sense to me to offer this as a general courtesy upstream, rather than just handling it for Fedora and derivatives downstream. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
participants (2)
-
Nick Coghlan
-
Victor Stinner