Skip to content

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

Merged
warsaw merged 12 commits intopython:masterfrom
warsaw:bpo-22807
Feb 18, 2017
Merged

bpo-22807: Expose platform UUID generation safety information.#138
warsaw merged 12 commits intopython:masterfrom
warsaw:bpo-22807

Conversation

@warsaw
Copy link
Copy Markdown
Member

@warsaw warsaw commented Feb 16, 2017

Add an enum SafeUUID which describes whether the platform has generated a UUID in a safe manner (i.e. ensuring that no two processes can generate the same UUID). Also add UUID.is_safe attribute which relays the information from the underlying platform.

Comment thread Lib/uuid.py
int &= ~(0xf000 << 64)
int |= version << 76
self.__dict__['int'] = int
self.__dict__['is_safe'] = is_safe
Copy link
Copy Markdown
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
Copy Markdown
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.

Comment thread Lib/uuid.py Outdated

def __init__(self, hex=None, bytes=None, bytes_le=None, fields=None,
int=None, version=None):
int=None, version=None, is_safe=SafeUUID.unknown):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I changed my mind on whether this should affect older versions of Python.

If so, you can also consider making it kwarg-only. As you are already using is as a kwarg. Not sure if that's useful, or worth it. It's may also be a question of taste, and leaving it as is seem perfectly fine.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for the review! I did think about that, and it might indeed make sense, but it's a matter of style in this case, and I wasn't sure I wanted to introduce that to the API. But on thinking about it again now that you bring it up, I does make sense.

Copy link
Copy Markdown
Contributor

@Carreau Carreau left a comment

Choose a reason for hiding this comment

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

Thanks for the review!

My pleasure !

I did think about that, and it might indeed make sense, but it's a matter of style in this case, and I wasn't sure I wanted to introduce that to the API. But on thinking about it again now that you bring it up, I does make sense.

Happy to be of some use :-)

Copy link
Copy Markdown
Member

@orsenthil orsenthil left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Copy Markdown
Member

@berkerpeksag berkerpeksag left a comment

Choose a reason for hiding this comment

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

I just left some comments, but the PR looks good to me.

Comment thread Doc/library/uuid.rst Outdated
.. class:: UUID(hex=None, bytes=None, bytes_le=None, fields=None, int=None, version=None)
.. class:: SafeUUID

.. attribute:: SafeUUID.safe
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know this is the current style in uuid.rst, but it might be better use the following style in new documentation:

.. class:: SafeUUID

   .. versionadded:: 3.7

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

   ...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, that looks nice, thanks. The one problem is that SafeUUID still gets labeled as a class in the HTML. Okay, technically it is a class, but it would be nice to have an enum:: markup. I grep'd around in the source and couldn't find any such examples, so I don't know if we support that yet.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You're right. We don't have an enum directive now, but it should be easy to add one. You can add me to nosy list if you create an issue on bugs.p.o :)

Comment thread Lib/test/test_uuid.py Outdated
equal(((u.clock_seq_hi_variant & 0x3f) << 8) |
u.clock_seq_low, 0x3fff)

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

Choose a reason for hiding this comment

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

Nitpick: I'd flip the order of the decorators.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

+1

Comment thread Lib/test/test_uuid.py
# unknown (unless I suppose the platform is buggy).
self.assertNotEqual(u.is_safe, uuid.SafeUUID.unknown)

@unittest.skipUnless(importable('ctypes'), 'requires ctypes')
Copy link
Copy Markdown
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
Copy Markdown
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.

Comment thread Doc/library/uuid.rst

.. versionadded:: 3.7

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

Choose a reason for hiding this comment

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

Currently, the body of UUID() documentation briefly describes all its arguments so I think we can just copy is_safe documentation in Lib/uuid.py

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

+1

``local.getlocale(locale.LC_CTYPE)`` and
``locale.getpreferredencoding(False)`` may give different answers
in some cases (such as the ``en_IN`` locale).

``re.LOCALE`` uses the latter, so update the test case to match.
@serhiy-storchaka serhiy-storchaka added the type-feature A feature request or enhancement label Feb 18, 2017
@warsaw warsaw merged commit 8c130d7 into python:master Feb 18, 2017
@warsaw warsaw deleted the bpo-22807 branch February 18, 2017 20:46
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 22, 2017
Replace STACKLESS_PROPOSE(func) in do_call() and ext_do_call() by
STACKLESS_PROPOSE_ALL().
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 29, 2017
- Initialize variable 'len' in function unwrap_frame_arg().
- Assign to '*valid' in function slp_find_execname().
- Disable GCC warning -Waddress in macro TASKLET_SETVAL(task, val).
- Update Stackless/changelog.txt for issues python#138, python#140
akruis pushed a commit to akruis/cpython that referenced this pull request Jun 19, 2018
Replace STACKLESS_PROPOSE(func) in do_call() and ext_do_call() by
STACKLESS_PROPOSE_ALL().
(cherry picked from commit ae3d7a2)
akruis pushed a commit to akruis/cpython that referenced this pull request Jun 19, 2018
- Assign to '*valid' in function slp_find_execname().
- Disable GCC warning -Waddress in macro TASKLET_SETVAL(task, val).
- Update Stackless/changelog.txt for issues python#138, python#140

(cherry picked from commit 3461ccf)
SonicField added a commit to SonicField/cpython that referenced this pull request Apr 25, 2026
…ary domains

W-PROTOCOL-CODIFY Phase A per supervisor 20:03:34Z directive (post pythia
python#138 python#3 + supervisor 19:37:39Z STRONG CONCUR + theologian 19:37:17Z spec).

Replaces named-only conversions (post W-2B-RECONVERT 3-class fix at
8e0c73e) with single-field struct typedefs that the C compiler tracks
distinctly. Inline arithmetic at the seam was the bug class, named
conversions were the WEAKER form (HirType negative control: 14
reinterpret_casts shipped broken until 2026-04-15 because naming
degrades to advisory). PhxMem has_base structural flag is the POSITIVE
control: JIT_CHECK_C didn't fire on dual-arch gate; structural prevention
worked.

CHANGES (Python/jit/bytecode_c.h):
- typedef struct { int v; } BcByteOffset;
- typedef struct { int v; } BcInstrIndex;
- phx_bc_offset_to_instr_index now takes BcByteOffset, returns BcInstrIndex
- phx_bc_instr_index_to_offset now takes BcInstrIndex, returns BcByteOffset
- Header comment cites P5 origin + paired empirical controls

CHANGES (Python/jit/hir/builder_emit_c.c, build_inline_except_opcode_array_c):
- Class A site: wrap except_body_offset in BcByteOffset, convert to
  BcInstrIndex, unwrap .v for jit_bc_instr_init (existing raw-int API)
- Class C site: wrap jit_bc_instr_base_offset return in BcInstrIndex,
  convert to BcByteOffset, unwrap .v for entry->base_offset
- Class B site: wrap jit_bc_instr_get_jump_target return in BcInstrIndex,
  convert to BcByteOffset, unwrap .v for phx_block_map_lookup_or_panic

The wrap/unwrap dance localizes the .v access to exactly the boundary
between raw-int existing APIs and typed conversion functions. Compiler
will catch any future cross-domain assignment (e.g., assigning
BcByteOffset to BcInstrIndex without conversion) as type error — the
structural property the spec requires.

Spec: docs/w-protocol-codify-spec.md P5 (lines 109-136).

Behavior: ZERO change. The numeric values are identical to the named-
conversion fix at 8e0c73e (same arithmetic, same call ordering). The
30/30 sentinels (test_exc_raise_catch + 3 W-2B-RECONVERT sentinels) +
test_multiple_exceptions_in_loop must remain 30/30 PASS.

OUT-OF-SCOPE for Phase A (per spec):
- Other domains (cache-id, ref-kind, type-kind) — apply pattern as
  encountered, not pre-authored.
- Existing C++ BCOffset/BCIndex code — already type-enforced in C++.
- jit_bc_instr_* APIs — keep raw int signatures; conversion happens at
  call sites in helpers that cross domains. Future Phase: if a new
  helper repeatedly wraps/unwraps the same value, consider promoting
  the return type to BcInstrIndex / BcByteOffset.

Phase B (gate integration) + Phase C (per-conversion P6/P7 application
to emitAnyCall + emitLoadMethodStatic) queued post-Phase-A.

Verification pending: testkeeper rebuild + 30x W-2B-RECONVERT sentinel
suite + 30x test_multiple_exceptions_in_loop + full Phoenix gate +
ABBA + dual-arch.
SonicField added a commit to SonicField/cpython that referenced this pull request Apr 25, 2026
Per supervisor 20:39:03Z directive (post pythia python#138 python#2: push 55 was the
first commit in 'ACTIVE-dispatch territory' — Class A unit bug previously
masked dispatch-loop coverage on multi-except paths via interpreter
fallback for the entire push-54 lifetime).

Adds 4 sentinels covering exception-handling shapes not exercised by the
W-2B-RECONVERT sentinels (which targeted Class A/B/C latent fixes):

1. test_exc_try_else — no-raise path + else branch. Exercises the
   no-fire route through emitInlineExceptionMatch fast-path + JIT_EVAL
   falls through to else block.

2. test_exc_try_finally_with_raise — nested try/finally inside try/except
   with raise(). The raise-CALL exercises emitCallExceptionHandler's D1
   stack-cleanup pre-amble + D3 result re-push invariant when an inner
   finally runs while the exception propagates to the outer try.

3. test_exc_nested_try_inner_catches — nested try where inner except
   catches and outer block continues normally. Exercises FrameState
   depth-trim correctness across nested handler scope transitions.

4. test_exc_raise_in_handler — re-raise a DIFFERENT exception type from
   inside an except handler. The new raise() inside except is itself
   a CALL (constructor of the new exception class), so
   emitCallExceptionHandler fires while we're already in handler dispatch
   — tests nested handler activation.

Tests 2-4 actively exercise emitCallExceptionHandler dispatch. Test 1
covers emitInlineExceptionMatch no-fire path. Together they cover the
four major dispatch shapes not exercised by W-2B-RECONVERT sentinels.

Per theologian 20:40:59Z patch-shape APPROVE. Verification pending:
testkeeper 30x each + regression check on existing 5 W-2B-RECONVERT
sentinels (helper unchanged at push 56).

If all 4 PASS → C dispatch path is sound for these shapes; W-2A-DISPATCH-
COVERAGE complete; pivot to W-PROTOCOL-CODIFY Phase B per roadmap.
If any FAIL → new bug discovery; investigate per HIR-diff Phase 0
methodology (compare to push-53 ebec018 reference for the failing
shape per theologian 20:40:59Z).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants