Repository navigation
Windows uuid.uuid1 is unecessarily unsafe for concurrent use #118993
Description
Bug report
Bug description:
Running uuid.uuid1 concurrently in multiple parallel python processes on Windows creates colliding UUIDs, due to the timestamps colliding. Can be seen with 10-20 concurent processes usually.
This is unnecessary, as python already wraps the UuidCreateSequential() API
Line 63 in 9c15202
, but then discards the time part and falls back to some collision prone time code in
Line 668 in b4ca389
But as far as i can tell, Windows UuidCreateSequential() does follow RFC 4122 by now, tested on Win10, so the comment looks wrong or badly aged. It is true, that the function returns the bytes in the wrong order, Little Endian, instead of the RFC prescribed Big Endian, but the rest looks sane.
This works just fine and creates a UUID thats pretty similar to the ones created by the current uuid.uuid1 implementation.
>>> import _uuid
>>> import uuid
>>> win_uuid = uuid.UUID(bytes_le=_uuid.UuidCreate())
>>> win_uuid.version
1
>>> win_uuid.variant
'specified in RFC 4122'
>>> win_uuid.time
139348915287723566
>>> win_uuid.node
185263057677337
>>> py_uuid = uuid.uuid1()
>>> py_uuid.version
1
>>> py_uuid.variant
'specified in RFC 4122'
>>> py_uuid.time
139348915744502628
>>> py_uuid.node
185263057677337I would propose to remove the unsafe time fallback for Windows and use the results of UuidCreate() to make it safe.
CPython versions tested on:
3.11
Operating systems tested on:
Windows
Activity
zooba commentedon Aug 7, 2024
Apologies for leaving this one for so long.
To clarify my own understanding, the reason this only affects Windows is because POSIX is always going to provide
_generate_time_safeand so won't go through the pure-Python calculation? (And so a system that doesn't provide that function will also be affected, even if it's not Windows.)It seems like
UuidCreateSequentialis the right alternative, though I'm a little surprised that we got to this point at all? That API has been around for a long time (the rewrite to use_uuidrather thanctypesis more recent, but that was a straight conversion and didn't involve any consideration for whether the results are RFC compliant or not).You just mean we need to construct with
bytes_le=rather thanbytes=, correct? It doesn't change the values compared to the current implementation? (It's compliant enough with the spec to do this, in that the RFC only specifies network order "in the absence" of another specification - Windows specifies that it uses native endian for each field, so it wins here.)If so, then the entire proposal is to simply add
if _UuidCreate is not None: return UUID(bytes_le=_UuidCreate())plus overwriting the two additional parameters if specified. Have I understood the situation well enough?schlenk commentedon Aug 8, 2024
Yes. If there is no HAVE_UUID_GENERATE_TIME_SAFE define, which seems to test some function from uuid.h, i think it would also affect other systems.
cpython/Modules/_uuidmodule.c
Line 101 in 9c15202
But the point is mostly that you have no alternatives on POSIX systems without that libuuid function, while you have a perfectly fine alternative on Win32 that is just not used.
This was already broken since forever, i had a local patch against some ancient Py 2.6 version, but assumed it got fixed in Py3, without checking at that time. And got surprised to find it still happening in Py3, when finally getting off the ancient codebase.
I saw a small time offset, see console listing above, which might be caused by different clocks used internally. So the pure python version creates slightly different timestamps than the UuidCreateSequential API. I would assume this does not cause an issue.
Yes.
This should also set the
is_safeparameter to SafeUUID.safe, as the UUIDs created by the UuidCreateSequential() are as safe as the ones by libuuid.Merge branch 'main' into pythongh-118993-windows-uuid1-safe