_random.Random state corrupted on exception

[bladebryan]

BPO	29960
Nosy	@rhettinger, @mdickinson, @serhiy-storchaka, @bladebryan, @Mariatta
PRs	bpo-29960 _random.Random state corrupted on exception. #953 bpo-29960 _random.Random corrupted on exception in setstate(). #1019 [3.6] bpo-29960 _random.Random corrupted on exception in setstate(). … #1287 [3.5] bpo-29960 _random.Random corrupted on exception in setstate(). … #1288 [2.7] bpo-29960 _random.Random corrupted on exception in setstate(). … #1289

^{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 = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2017-05-27.14:22:10.000>
created_at = <Date 2017-04-01.10:46:26.025>
labels = ['3.7', 'type-bug', 'library']
title = '_random.Random state corrupted on exception'
updated_at = <Date 2017-05-27.14:22:09.999>
user = 'https://github.com/bladebryan'

bugs.python.org fields:

activity = <Date 2017-05-27.14:22:09.999>
actor = 'Mariatta'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-05-27.14:22:10.000>
closer = 'Mariatta'
components = ['Library (Lib)']
creation = <Date 2017-04-01.10:46:26.025>
creator = 'bryangeneolson'
dependencies = []
files = []
hgrepos = []
issue_num = 29960
keywords = []
message_count = 8.0
messages = ['290977', '290979', '292108', '293927', '294590', '294591', '294592', '294593']
nosy_count = 5.0
nosy_names = ['rhettinger', 'mark.dickinson', 'serhiy.storchaka', 'bryangeneolson', 'Mariatta']
pr_nums = ['953', '1019', '1287', '1288', '1289']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue29960'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6', 'Python 3.7']

[bladebryan]

Demo:

Run the Python library's test_random.py under the Python debugger and check the
generator at the start of test_shuffle():

C:\bin\Python36>python -m pdb Lib\test\test_random.py

c:\bin\python36\lib\test\test_random.py(1)<module>()
-> import unittest
(Pdb) break 61
Breakpoint 1 at c:\bin\python36\lib\test\test_random.py:61
(Pdb) continue
.............................> c:\bin\python36\lib\test\test_random.py(61)test_shuffle()
-> shuffle = self.gen.shuffle
(Pdb) list
56 # randomness source is not available.
57 urandom_mock.side_effect = NotImplementedError
58 self.test_seedargs()
59
60 def test_shuffle(self):
61 B-> shuffle = self.gen.shuffle
62 lst = []
63 shuffle(lst)
64 self.assertEqual(lst, [])
65 lst = [37]
66 shuffle(lst)
(Pdb) p self.gen.getrandbits(31)
2137781566
(Pdb) p self.gen.getrandbits(31)
2137781566
(Pdb) p self.gen.getrandbits(31)
2137781566
(Pdb) p self.gen.getrandbits(31)
2137781566
(Pdb) p self.gen.getrandbits(31)
2137781566

That's not random.

Diagnosis:

The order in which test functions run is the lexicographic order of their names. Thus unittest ran test_setstate_middle_arg() before running test_shuffle(). test_setstate_middle_arg() did some failed calls to _random.Random.setstate(), which raised exceptions as planned, but also trashed the state of the generator. test_random.py continues to use the same instance of _random.Random after setstate() raises exceptions.

The documentation for Random.setstate() does not specify what happens to the state of the generator if setstate() raises an exception. Fortunately the generator recommended for secure applications, SystemRandom, does not implement setstate().

Solution:

The fix I prefer is a small change to random_setstate() in _randommodule.c, so that it does not change the state of the generator until the operation is sure to succeed.

[bladebryan]

I'm going through https://docs.python.org/devguide/pullrequest.html and would like to be assigned this issue.

[serhiy-storchaka]

New changeset 9616a82 by Serhiy Storchaka (bladebryan) in branch 'master':
bpo-29960 _random.Random corrupted on exception in setstate(). (bpo-1019)
9616a82

[serhiy-storchaka]

Mariatta, can you update your PRs?

[Mariatta]

New changeset 440bc4f by Mariatta in branch '3.5':
[3.5] bpo-29960 _random.Random corrupted on exception in setstate(). … (bpo-1288)
440bc4f

[Mariatta]

New changeset 1626a47 by Mariatta in branch '2.7':
[2.7] bpo-29960 _random.Random corrupted on exception in setstate(). … (bpo-1289)
1626a47

[Mariatta]

New changeset 94d8261 by Mariatta in branch '3.6':
[3.6] bpo-29960 _random.Random corrupted on exception in setstate(). … (bpo-1287)
94d8261

[Mariatta]

Thanks Bryan and Serhiy.