random.py still broken wrt. urandom
I consider the random module still broken in its current form (1.66). It tries to invoke random.urandom(1) in order to find out whether urandom works. Instead, it should defer that determination until urandom is actually used; i.e. instead of if _urandom is None: import time a = long(time.time() * 256) # use fractional seconds else: a = long(_hexlify(_urandom(16)), 16) it should read try: a = long(_hexlify(os.urandom(16)), 16) except NotImplementedError: import time a = long(time.time() * 256) # use fractional seconds IMO the patch to random.py should not have been applied without a review. Regards, Martin
[Martin v. Löwis]
I consider the random module still broken in its current form (1.66). It tries to invoke random.urandom(1) in order to find out whether urandom works. Instead, it should defer that determination until urandom is actually used;
Why?
i.e. instead of
if _urandom is None: import time a = long(time.time() * 256) # use fractional seconds else: a = long(_hexlify(_urandom(16)), 16)
it should read
try: a = long(_hexlify(os.urandom(16)), 16) except NotImplementedError: import time a = long(time.time() * 256) # use fractional seconds
Why? I like it better the way it is, in part because this kind of determination is made at least 4 times in random.py, and the "_urandom is None" spelling is quite clear. The from binascii import hexlify as _hexlify import certainly doesn't belong in the try/except block setting that up, though.
IMO the patch to random.py should not have been applied without a review.
I think that falls under the "expert rule": Raymond has done more work on random.py than everyone else combined over the last year or two, and he had no reason to suspect this change would be controversial. To the contrary, I specifically suggested (on python-dev) that using urandom in seed() methods, when available, would be a significant improvement over time.time()-based seeding. Now that you've made your objection, I confess I still have no idea why you're objecting (see "why?" <wink>). I did review the patch (after the fact) for numeric correctness (which did lead to changing the code, due to a subtle numeric flaw in the original HardwareRandom.random).
Tim Peters wrote:
I consider the random module still broken in its current form (1.66). It tries to invoke random.urandom(1) in order to find out whether urandom works. Instead, it should defer that determination until urandom is actually used;
Why?
Invoking urandom() causes /dev/urandom to be opened on Unix (if available). I really would prefer if merely importing the random module would not open files (and keep them open for the entire Python run). Operations like this contribute to startup time. Now, since the random module also creates a Random object, importing random would still open /dev/urandom even if the logic dealing with its absence was somewhat deferred - unless seeding the RNG would also be deferred until it is first used. Still, consuming randomness just to determine whether it is available seems wrong. Importing a module should not affect system state unless absolutely necessary.
I think that falls under the "expert rule": Raymond has done more work on random.py than everyone else combined over the last year or two, and he had no reason to suspect this change would be controversial. To the contrary, I specifically suggested (on python-dev) that using urandom in seed() methods, when available, would be a significant improvement over time.time()-based seeding.
I have no problem with that aspect of the change. However, I wish somebody had noticed that unavailability of urandom is expressed through a NotImplementedError, not through absence of the function itself. It is bad luck that this state of the code was released as 2.4a3. Regards, Martin
participants (2)
-
"Martin v. Löwis" -
Tim Peters