Skip to content

bpo-22807: Expose platform UUID generation safety information. #138

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.

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 12 commits into from
Feb 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 30 additions & 1 deletion Doc/library/uuid.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,30 @@ If all you want is a unique ID, you should probably call :func:`uuid1` or
a UUID containing the computer's network address. :func:`uuid4` creates a
random UUID.

Depending on support from the underlying platform, :func:`uuid1` may or may
not return a "safe" UUID. A safe UUID is one which is generated using
synchronization methods that ensure no two processes can obtain the same
UUID. All instances of :class:`UUID` have an :attr:`is_safe` attribute
which relays any information about the UUID's safety, using this enumeration:

.. class:: UUID(hex=None, bytes=None, bytes_le=None, fields=None, int=None, version=None)
.. class:: SafeUUID

.. versionadded:: 3.7

.. attribute:: SafeUUID.safe

The UUID was generated by the platform in a multiprocessing-safe way.

.. attribute:: SafeUUID.unsafe

The UUID was not generated in a multiprocessing-safe way.

.. attribute:: SafeUUID.unknown

The platform does not provide information on whether the UUID was
generated safely or not.

.. class:: UUID(hex=None, bytes=None, bytes_le=None, fields=None, int=None, version=None, *, is_safe=SafeUUID.unknown)

Create a UUID from either a string of 32 hexadecimal digits, a string of 16
bytes as the *bytes* argument, a string of 16 bytes in little-endian order as
Expand Down Expand Up @@ -120,6 +142,13 @@ random UUID.
The UUID version number (1 through 5, meaningful only when the variant is
:const:`RFC_4122`).

.. attribute:: UUID.is_safe

An enumeration of :class:`SafeUUID` which indicates whether the platform
generated the UUID in a multiprocessing-safe way.

.. versionadded:: 3.7

The :mod:`uuid` module defines the following functions:


Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_re.py
Original file line number Diff line number Diff line change
Expand Up @@ -1399,7 +1399,7 @@ def test_ascii_and_unicode_flag(self):

def test_locale_flag(self):
import locale
_, enc = locale.getlocale(locale.LC_CTYPE)
enc = locale.getpreferredencoding(False)
# Search non-ASCII letter
for i in range(128, 256):
try:
Expand Down
40 changes: 40 additions & 0 deletions Lib/test/test_uuid.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,46 @@ def test_uuid1(self):
equal(((u.clock_seq_hi_variant & 0x3f) << 8) |
u.clock_seq_low, 0x3fff)

@unittest.skipUnless(uuid._uuid_generate_time.restype is not None,
'requires uuid_generate_time_safe(3)')
@unittest.skipUnless(importable('ctypes'), 'requires ctypes')
def test_uuid1_safe(self):
u = uuid.uuid1()
# uuid_generate_time_safe() may return 0 or -1 but what it returns is
# dependent on the underlying platform support. At least it cannot be
# unknown (unless I suppose the platform is buggy).
self.assertNotEqual(u.is_safe, uuid.SafeUUID.unknown)

@unittest.skipUnless(importable('ctypes'), 'requires ctypes')
Copy link
Member

Choose a reason for hiding this comment

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

Just a suggestion: Using self.subTest() would make the following four tests much shorter and IMO easier to understand.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've played a bit with subTest() and I'm not entirely thrilled with it. AFAICT, subtests only print output when they fail, not when they succeed. In verbose mode, I'd want to see all 4 tests printed individually. (That's one of the things I'd like to eventually fix.) For now, I think I'll leave them as separate tests.

def test_uuid1_unknown(self):
# Even if the platform has uuid_generate_time_safe(), let's mock it to
# be uuid_generate_time() and ensure the safety is unknown.
with unittest.mock.patch.object(uuid._uuid_generate_time,
'restype', None):
u = uuid.uuid1()
self.assertEqual(u.is_safe, uuid.SafeUUID.unknown)

@unittest.skipUnless(importable('ctypes'), 'requires ctypes')
def test_uuid1_is_safe(self):
with unittest.mock.patch.object(uuid._uuid_generate_time,
'restype', lambda x: 0):
u = uuid.uuid1()
self.assertEqual(u.is_safe, uuid.SafeUUID.safe)

@unittest.skipUnless(importable('ctypes'), 'requires ctypes')
def test_uuid1_is_unsafe(self):
with unittest.mock.patch.object(uuid._uuid_generate_time,
'restype', lambda x: -1):
u = uuid.uuid1()
self.assertEqual(u.is_safe, uuid.SafeUUID.unsafe)

@unittest.skipUnless(importable('ctypes'), 'requires ctypes')
def test_uuid1_bogus_return_value(self):
with unittest.mock.patch.object(uuid._uuid_generate_time,
'restype', lambda x: 3):
u = uuid.uuid1()
self.assertEqual(u.is_safe, uuid.SafeUUID.unknown)

def test_uuid3(self):
equal = self.assertEqual

Expand Down
43 changes: 37 additions & 6 deletions Lib/uuid.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@

import os

from enum import Enum


__author__ = 'Ka-Ping Yee <ping@zesty.ca>'

RESERVED_NCS, RFC_4122, RESERVED_MICROSOFT, RESERVED_FUTURE = [
Expand All @@ -55,7 +58,14 @@
int_ = int # The built-in int type
bytes_ = bytes # The built-in bytes type

class UUID(object):

class SafeUUID(Enum):
safe = 0
unsafe = -1
unknown = None


class UUID:
"""Instances of the UUID class represent UUIDs as specified in RFC 4122.
UUID objects are immutable, hashable, and usable as dictionary keys.
Converting a UUID to a string with str() yields something in the form
Expand Down Expand Up @@ -101,10 +111,15 @@ class UUID(object):

version the UUID version number (1 through 5, meaningful only
when the variant is RFC_4122)

is_safe An enum indicating whether the UUID has been generated in
a way that is safe for multiprocessing applications, via
uuid_generate_time_safe(3).
"""

def __init__(self, hex=None, bytes=None, bytes_le=None, fields=None,
int=None, version=None):
int=None, version=None,
*, is_safe=SafeUUID.unknown):
r"""Create a UUID from either a string of 32 hexadecimal digits,
a string of 16 bytes as the 'bytes' argument, a string of 16 bytes
in little-endian order as the 'bytes_le' argument, a tuple of six
Expand All @@ -128,6 +143,10 @@ def __init__(self, hex=None, bytes=None, bytes_le=None, fields=None,
be given. The 'version' argument is optional; if given, the resulting
UUID will have its variant and version set according to RFC 4122,
overriding the given 'hex', 'bytes', 'bytes_le', 'fields', or 'int'.

is_safe is an enum exposed as an attribute on the instance. It
indicates whether the UUID has been generated in a way that is safe
for multiprocessing applications, via uuid_generate_time_safe(3).
"""

if [hex, bytes, bytes_le, fields, int].count(None) != 4:
Expand Down Expand Up @@ -182,6 +201,7 @@ def __init__(self, hex=None, bytes=None, bytes_le=None, fields=None,
int &= ~(0xf000 << 64)
int |= version << 76
self.__dict__['int'] = int
self.__dict__['is_safe'] = is_safe
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to assert the type of is_safe ? There seem to extensive checking of the rest of the parameters.
Not that I imagine people creating UUID object in their own code, but is_safe feel also a lot like a Boolean.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure it's worth type checking is_safe. The other ones are checked to ensure sane, compliant UUIDs, but is_safe is just a flag with some extra information (which may or may not be helpful to an application). If someone wants to manually instantiate UUIDs with bogus is_safe flags, oh well.


def __eq__(self, other):
if isinstance(other, UUID):
Expand Down Expand Up @@ -472,10 +492,17 @@ def _netbios_getnode():
for libname in _libnames:
try:
lib = ctypes.CDLL(ctypes.util.find_library(libname))
except Exception:
except Exception: # pragma: nocover
continue
if hasattr(lib, 'uuid_generate_time'):
# Try to find the safe variety first.
if hasattr(lib, 'uuid_generate_time_safe'):
_uuid_generate_time = lib.uuid_generate_time_safe
# int uuid_generate_time_safe(uuid_t out);
break
elif hasattr(lib, 'uuid_generate_time'): # pragma: nocover
_uuid_generate_time = lib.uuid_generate_time
# void uuid_generate_time(uuid_t out);
_uuid_generate_time.restype = None
break
del _libnames

Expand Down Expand Up @@ -566,8 +593,12 @@ def uuid1(node=None, clock_seq=None):
# use UuidCreate here because its UUIDs don't conform to RFC 4122).
if _uuid_generate_time and node is clock_seq is None:
_buffer = ctypes.create_string_buffer(16)
_uuid_generate_time(_buffer)
return UUID(bytes=bytes_(_buffer.raw))
safely_generated = _uuid_generate_time(_buffer)
try:
is_safe = SafeUUID(safely_generated)
except ValueError:
is_safe = SafeUUID.unknown
return UUID(bytes=bytes_(_buffer.raw), is_safe=is_safe)

global _last_timestamp
import time
Expand Down
9 changes: 9 additions & 0 deletions Misc/NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,10 @@ Extension Modules
Library
-------

- bpo-22807: Add uuid.SafeUUID and uuid.UUID.is_safe to relay information from
the platform about whether generated UUIDs are generated with a
multiprocessing safe method.

- bpo-29576: Improve some deprecations in importlib. Some deprecated methods
now emit DeprecationWarnings and have better descriptive messages.

Expand Down Expand Up @@ -801,6 +805,11 @@ Tools/Demos
Tests
-----

- Issue #29571: to match the behaviour of the ``re.LOCALE`` flag,
test_re.test_locale_flag now uses ``locale.getpreferredencoding(False)`` to
determine the candidate encoding for the test regex (allowing it to correctly
skip the test when the default locale encoding is a multi-byte encoding)

- Issue #24932: Use proper command line parsing in _testembed

- Issue #28950: Disallow -j0 to be combined with -T/-l in regrtest
Expand Down
11 changes: 7 additions & 4 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ reserved.

See the end of this file for further copyright and license information.

Contributing to CPython
-----------------------

For more complete instructions on contributing to CPython development,
see the `Developer Guide`_.

.. _Developer Guide: https://docs.python.org/devguide/

Using Python
------------
Expand Down Expand Up @@ -126,10 +133,6 @@ is downloadable in HTML, PDF, and reStructuredText formats; the latter version
is primarily for documentation authors, translators, and people with special
formatting requirements.

If you would like to contribute to the development of Python, relevant
documentation is available in the `Python Developer's Guide
<https://docs.python.org/devguide/>`_.

For information about building Python's documentation, refer to `Doc/README.rst
<https://github.com/python/cpython/blob/master/Doc/README.rst>`_.

Expand Down