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

Created on 2017-04-01 10:46 by bryangeneolson, last changed 2017-05-18 15:16 by serhiy.storchaka.

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 open Mariatta, 2017-04-26 02:35
PR 1288 open Mariatta, 2017-04-26 02:38
PR 1289 open Mariatta, 2017-04-26 02:40
Messages (4)
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?
History
Date User Action Args
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