Skip to content

bpo-29960 _random.Random corrupted on exception in setstate(). #1019

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 22, 2017
Merged

bpo-29960 _random.Random corrupted on exception in setstate(). #1019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d6a1a10
Issue29960 _random.Random state corrupted on exception.
bladebryan Apr 7, 2017
0a2bca9
bpo-29960 Fixed whitespace and attribution as per code review.
bladebryan Apr 7, 2017
e0f4bab
Merge branch 'master' into randstate
serhiy-storchaka Apr 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions Lib/test/test_random.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ def test_setstate_first_arg(self):
self.assertRaises(ValueError, self.gen.setstate, (1, None, None))

def test_setstate_middle_arg(self):
start_state = self.gen.getstate()
# Wrong type, s/b tuple
self.assertRaises(TypeError, self.gen.setstate, (2, None, None))
# Wrong length, s/b 625
Expand All @@ -436,6 +437,10 @@ def test_setstate_middle_arg(self):
self.gen.setstate((2, (1,)*624+(625,), None))
with self.assertRaises((ValueError, OverflowError)):
self.gen.setstate((2, (1,)*624+(-1,), None))
# Failed calls to setstate() should not have changed the state.
bits100 = self.gen.getrandbits(100)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that the test is correct. Why not generating bits before calling setstate()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm testing that the failed calls to setstate() did not change the state. I save the initial state, then the failed calls happen. Now I want to know if I'm still in the initial state. I don't check for state equality because a generator's state might not support equality test. I test by generating bits, restoring the initial state, and generating what should be the same bits.
I'm not testing the succeeding call to setstate(). There are other test cases for that, so I assume it works.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm that the test is correct.

self.gen.setstate(start_state)
self.assertEqual(self.gen.getrandbits(100), bits100)

# Little trick to make "tuple(x % (2**32) for x in internalstate)"
# raise ValueError. I cannot think of a simple way to achieve this, so
Expand Down
1 change: 1 addition & 0 deletions Misc/ACKS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1110,6 +1110,7 @@ Milan Oberkirch
Pascal Oberndoerfer
Jeffrey Ollie
Adam Olsen
Bryan Olson
Grant Olson
Koray Oner
Piet van Oostrum
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,9 @@ Extension Modules
Library
-------

- bpo-29960: Preserve generator state when _random.Random.setstate()
raises an exception. Patch by Bryan Olson.

- bpo-29802: Fixed reference counting in module-level struct functions when
pass arguments of wrong type.

Expand Down
5 changes: 4 additions & 1 deletion Modules/_randommodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ random_setstate(RandomObject *self, PyObject *state)
int i;
unsigned long element;
long index;
uint32_t new_state[N];

if (!PyTuple_Check(state)) {
PyErr_SetString(PyExc_TypeError,
Expand All @@ -364,7 +365,7 @@ random_setstate(RandomObject *self, PyObject *state)
element = PyLong_AsUnsignedLong(PyTuple_GET_ITEM(state, i));
if (element == (unsigned long)-1 && PyErr_Occurred())
return NULL;
self->state[i] = (uint32_t)element;
new_state[i] = (uint32_t)element;
}

index = PyLong_AsLong(PyTuple_GET_ITEM(state, i));
Expand All @@ -375,6 +376,8 @@ random_setstate(RandomObject *self, PyObject *state)
return NULL;
}
self->index = (int)index;
for (i = 0; i < N; i++)
self->state[i] = new_state[i];

Py_RETURN_NONE;
}
Expand Down