dep-protobuf/python/google
Joshua Haberman 8c1a9a4b01 Fixed data race in Python Free Threading by removing unnecessary SetHasBitForRepeated() call.
This CL concerns the following functions in C++:

```c++
class Reflection {
  const Message& GetRepeatedMessage(const Message& message,
                                    const FieldDescriptor* field,
                                    int index) const;

  Message* MutableRepeatedMessage(Message* message,
                                  const FieldDescriptor* field,
                                  int index) const;
}
```

Suppose a Python program contains the following code:

```
def foo(msg: MyMessage):
  submsg = msg.repeated_foo[2]  # (A)
  if also_mutate:
    submsg.abc = 1              # (B)
```

For line (A), we currently call `MutableRepeatedMessage()` in C++ to
obtain the submessage pointer, because the resulting `submsg` object in
Python is a conceptually mutable object that will require a non-const
pointer if/when the program hits line (B).

The `MutableRepeatedMessage(Message* msg, ...)` function in C++
currently mutates `msg` by setting the hasbit of the repeated field. But
this seems unnecessary, as the function requires that the requested
sub-message already exists; it does not create a new message. So it
should not be necessary to touch the hasbit of the message, and a TGP
confirms that all tests pass if we remove this call.

Once the `SetHasBitForRepeated()` call is removed, the `MutableRepeatedMessage()`
function no longer actually mutates the `msg` argument. This makes it
effectively safe to call concurrently (ie. it will no longer trigger
Undefined Behavior in C++, and the TSAN errors will go away), which
is why it fixes the free threading test. But it leaves us in an odd
state where we are still passing a non-`const` pointer to the same
message to two functions concurrently, which is not allowed under a
normal thread-compatible contract.

An alternate solution would be to call `GetRepeatedMessage()` instead,
and `const_cast<>` away the const in the returned `const Message&`.
But this is also violates the contract: users should not be casting
away `const`.

The root cause of this odd situation is that
`MutableRepeatedMessage(Message* msg)` requires a non-`const` `msg` not
because it actually *mutates* `msg`, but because it is trying to
propagate the const-ness of `msg` to all of its children.  The proto API
generally guarantees that a const message prevents mutation of not just
the top-level message, but of the entire tree of messages. If someone
passes you a `const` message pointer, that is supposed to render the
entire tree of messages under it immutable through that pointer.

What we wish we could express in `MutableRepeatedMessage(Message* msg)`
is: "this function requires that `msg` is mutable, but this function
will not actually mutate it, and is therefore safe to call concurrently."
But there is no way of expressing this in C++.
PiperOrigin-RevId: 886830933
2026-03-20 09:29:13 -07:00
..
protobuf Fixed data race in Python Free Threading by removing unnecessary SetHasBitForRepeated() call. 2026-03-20 09:29:13 -07:00
__init__.py drop-deprecated-pkg-resources-declare (#22442) 2025-07-11 11:09:55 -07:00