classification
Title: Python 3.6 on Windows doesn't seed Random() well enough
Type: Stage: resolved
Components: Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: altendky, ammar2, brett.cannon, dstufft, eryksun, haypo, nedbat, paul.moore, python-dev, rhettinger, steve.dower, tim.golden, zach.ware
Priority: normal Keywords: 3.6regression

Created on 2016-12-27 17:39 by nedbat, last changed 2017-01-09 10:08 by haypo. This issue is now closed.

Messages (10)
msg284118 - (view) Author: Ned Batchelder (nedbat) * Date: 2016-12-27 17:39
Creating two Random() instances in quick succession produces the same sequence, but only on Windows on Python 3.6.  On 3.5 or earlier, or on Mac/Linux, the randomization is good.

Python 3.6.0 (v3.6.0:41df79263a11, Dec 23 2016, 07:18:10) [MSC v.1900 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import random; print(random.Random().randint(1, 999999), random.Random().randint(1, 999999))
903885 903885
>>> import random; print(*[random.Random().randint(1, 999999) for _ in range(2)])
996947 56476
>>> import random; print(*[random.Random().randint(1, 999999) for _ in range(2)])
793282 793282
>>> import random; print(*[random.Random().randint(1, 999999) for _ in range(2)])
519702 519702
>>> import random; print(*[random.Random().randint(1, 999999) for _ in range(2)])
230678 230678
>>> import random; print(*[random.Random().randint(1, 999999) for _ in range(3)])
474701 474701 474701
>>> import random; print(*[random.Random().randint(1, 999999) for _ in range(3)])
890942 890942 890942
>>> import random; print(*[random.Random().randint(1, 999999) for _ in range(3)])
997495 997495 997495
>>> import random; print(*[random.Random().randint(1, 999999) for _ in range(5)])
27803 27803 27803 27803 919401
>>>

I would expect each of these runs to produce unique numbers, with no duplicates.
msg284120 - (view) Author: Ned Batchelder (nedbat) * Date: 2016-12-27 18:12
Adding a time.sleep(0.001) (or maybe even just 0) between the calls prevents the problem.
msg284121 - (view) Author: Donald Stufft (dstufft) * (Python committer) Date: 2016-12-27 18:17
It's presumably failing to read nonblocking random from CrpytGen and falling back to seeding using time and pid.
msg284122 - (view) Author: Ammar Askar (ammar2) * Date: 2016-12-27 18:20
Can recreate under

Python 3.6.0 (v3.6.0:41df79263a11, Dec 23 2016, 08:06:12) [MSC v.1900 64 bit (AMD64)] on win32

and on latest master.
msg284123 - (view) Author: Kyle Altendorf (altendky) * Date: 2016-12-27 18:42
time.sleep(0) and time.sleep(0.0) acted the same for me and both exhibited matching 'random' values in some cases.  (win10, python3.6)  I also printed time.time() with each 'random' value and it wasn't a perfect match but matching times tended to go with matching 'random' values.
msg284127 - (view) Author: Eryk Sun (eryksun) * Date: 2016-12-27 19:20
> It's presumably failing to read nonblocking random from CrpytGen 
> and falling back to seeding using time and pid.

Yes and no. CryptGenRandom is not failing, but it is nonetheless calling random_seed_time_pid:

    >>> r = random.Random()
    Breakpoint 0 hit
    python36_d!random_seed:
    00000000`5e7277c0 4889542410      mov     qword ptr [rsp+10h],rdx
                                        ss:00000095`053ef0e8=0000000000000000
    0:000> g
    Breakpoint 1 hit
    python36_d!random_seed_time_pid:
    00000000`5e728910 48894c2408      mov     qword ptr [rsp+8],rcx
                                        ss:00000095`053ef040=000002319b6f3b88
    0:000> kc 3
    Call Site
    python36_d!random_seed_time_pid
    python36_d!random_seed
    python36_d!random_new

It looks like there's a bug in the new implementation of random_seed():

     if (arg == NULL || arg == Py_None) {
        if (random_seed_urandom(self) >= 0) {
            PyErr_Clear();

            /* Reading system entropy failed, fall back on the worst entropy:
               use the current time and process identifier. */
            random_seed_time_pid(self);
        }
        Py_RETURN_NONE;
    }

If random_seed_urandom fails, it returns -1, so this should be testing if the result is less than 0.
msg284192 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-12-28 19:42
Does this affect the secrets module at all?
msg284194 - (view) Author: Eryk Sun (eryksun) * Date: 2016-12-28 20:08
The secrets module uses SystemRandom, which overrides random() and getrandbits() to use os.urandom, so it's not affected.
msg284225 - (view) Author: Roundup Robot (python-dev) Date: 2016-12-29 04:03
New changeset 0a55e039d25f by Benjamin Peterson in branch '3.6':
fix error check, so that Random.seed actually uses OS randomness (closes #29085)
https://hg.python.org/cpython/rev/0a55e039d25f

New changeset fc3eab44765f by Benjamin Peterson in branch 'default':
merge 3.6 (#29085)
https://hg.python.org/cpython/rev/fc3eab44765f
msg285031 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-01-09 10:08
Oh, sorry, I introduced the bug in Python 3.6 with the PEP 524 (os.urandom() now blocks on Linux). Too bad that there is no simple way to write an unit test for that.

> ... but only on Windows on Python 3.6

With the bug, or when the fix when _PyOS_URandomNonblock() fails, Random.seed() uses:

* Reading system entropy failed, fall back on the worst entropy:
  use the current time and process identifier. */
random_seed_time_pid(self);

It's just that on Windows, the system clock has a resolution around 15 ms, whereas it has a resolution better than 1 us on Linux. So it's just that calling Random.seed() usually takes longer than the resolution of the system clock on Linux :-) Not really that the bug is specific to Windows.

Thanks for the fix Benjamin!
History
Date User Action Args
2017-01-09 10:08:17hayposetnosy: + haypo
messages: + msg285031
2017-01-09 00:14:06martin.panterlinkissue29208 superseder
2016-12-29 04:03:32python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg284225

resolution: fixed
stage: resolved
2016-12-28 20:08:13eryksunsetmessages: + msg284194
2016-12-28 19:42:38brett.cannonsetnosy: + paul.moore, tim.golden, zach.ware, steve.dower
2016-12-28 19:42:16brett.cannonsetnosy: + brett.cannon
messages: + msg284192
2016-12-27 19:23:43serhiy.storchakasetassignee: rhettinger

nosy: + rhettinger
2016-12-27 19:20:33eryksunsetnosy: + eryksun
messages: + msg284127
2016-12-27 18:42:39altendkysetmessages: + msg284123
2016-12-27 18:20:04ammar2setnosy: + ammar2

messages: + msg284122
versions: + Python 3.7
2016-12-27 18:18:00dstufftsetnosy: + dstufft
messages: + msg284121
2016-12-27 18:12:43nedbatsetmessages: + msg284120
2016-12-27 17:42:02altendkysetnosy: + altendky
2016-12-27 17:40:17nedbatsetversions: + Python 3.6
2016-12-27 17:39:24nedbatcreate