classification
Title: _random.Random state corrupted on exception
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Mariatta, bryangeneolson, mark.dickinson, rhettinger, serhiy.storchaka
Priority: normal Keywords:

Created on 2017-04-01 10:46 by bryangeneolson, last changed 2017-05-27 14:22 by Mariatta. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 953 closed bryangeneolson, 2017-04-01 21:05
PR 1019 merged bryangeneolson, 2017-04-07 06:23
PR 1287 merged Mariatta, 2017-04-26 02:35
PR 1288 merged Mariatta, 2017-04-26 02:38
PR 1289 merged Mariatta, 2017-04-26 02:40
Messages (8)
msg290977 - (view) Author: Bryan G. Olson (bryangeneolson) * Date: 2017-04-01 10:46
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.
msg290979 - (view) Author: Bryan G. Olson (bryangeneolson) * Date: 2017-04-01 10:50
I'm going through https://docs.python.org/devguide/pullrequest.html and would like to be assigned this issue.
msg292108 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-04-22 06:10
New changeset 9616a82e7802241a4b74cf7ae38d43c37bf66e48 by Serhiy Storchaka (bladebryan) in branch 'master':
bpo-29960 _random.Random corrupted on exception in setstate(). (#1019)
https://github.com/python/cpython/commit/9616a82e7802241a4b74cf7ae38d43c37bf66e48
msg293927 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-05-18 15:16
Mariatta, can you update your PRs?
msg294590 - (view) Author: Mariatta Wijaya (Mariatta) * (Python committer) Date: 2017-05-27 14:19
New changeset 440bc4f4b2690b99541e87bedfdb0dc4abbc0501 by Mariatta in branch '3.5':
[3.5] bpo-29960 _random.Random corrupted on exception in setstate(). … (#1288)
https://github.com/python/cpython/commit/440bc4f4b2690b99541e87bedfdb0dc4abbc0501
msg294591 - (view) Author: Mariatta Wijaya (Mariatta) * (Python committer) Date: 2017-05-27 14:19
New changeset 1626a479e22db3d44bcd6736d571243433cb6d0e by Mariatta in branch '2.7':
[2.7] bpo-29960 _random.Random corrupted on exception in setstate(). … (#1289)
https://github.com/python/cpython/commit/1626a479e22db3d44bcd6736d571243433cb6d0e
msg294592 - (view) Author: Mariatta Wijaya (Mariatta) * (Python committer) Date: 2017-05-27 14:20
New changeset 94d8261d1ca3f0c551d9d43a4db342d806af7a6a by Mariatta in branch '3.6':
[3.6] bpo-29960 _random.Random corrupted on exception in setstate(). … (#1287)
https://github.com/python/cpython/commit/94d8261d1ca3f0c551d9d43a4db342d806af7a6a
msg294593 - (view) Author: Mariatta Wijaya (Mariatta) * (Python committer) Date: 2017-05-27 14:22
Thanks Bryan and Serhiy.
History
Date User Action Args
2017-05-27 14:22:10Mariattasetstatus: open -> closed
resolution: fixed
messages: + msg294593

stage: backport needed -> resolved
2017-05-27 14:20:26Mariattasetmessages: + msg294592
2017-05-27 14:19:57Mariattasetmessages: + msg294591
2017-05-27 14:19:42Mariattasetnosy: + Mariatta
messages: + msg294590
2017-05-18 15:16:49serhiy.storchakasetmessages: + msg293927
2017-04-26 02:40:07Mariattasetpull_requests: + pull_request1397
2017-04-26 02:38:15Mariattasetpull_requests: + pull_request1396
2017-04-26 02:35:33Mariattasetpull_requests: + pull_request1395
2017-04-22 06:13:14serhiy.storchakasetassignee: serhiy.storchaka
stage: needs patch -> backport needed
versions: + Python 2.7, Python 3.5, Python 3.6
2017-04-22 06:10:48serhiy.storchakasetmessages: + msg292108
2017-04-07 06:23:23bryangeneolsonsetpull_requests: + pull_request1181
2017-04-05 06:23:24rhettingersetnosy: + serhiy.storchaka
2017-04-05 06:23:03rhettingersetassignee: rhettinger -> (no value)
2017-04-01 21:05:19bryangeneolsonsetpull_requests: + pull_request1135
2017-04-01 15:43:39rhettingersetassignee: rhettinger
2017-04-01 11:57:56serhiy.storchakasetnosy: + rhettinger, mark.dickinson

stage: needs patch
2017-04-01 10:50:49bryangeneolsonsetmessages: + msg290979
2017-04-01 10:46:26bryangeneolsoncreate