gh-158600: Fix a race condition in concurrent set.intersection_update()

[CaQtiml]

In the original implementation of set_intersection_update_multi_impl, a race condition can happen across several threads since a calculation and assignment phase have a critical section gap, so it is possible that an intersection result is lost. This new implementation includes both phases in a critical section.

I use set_copy_untracked_lock_held to create a copy used by set_intersection_multi_impl because so is held by Py_BEGIN_CRITICAL_SECTION, so I need to create another shallow copy of the set to use for the intersection.

To test if my implementation is correct, I run regression tests on Lib/test/test_set.py and Lib/test/test_free_threading/test_set.py and see no issues. I add two more tests to Lib/test/test_free_threading/test_set.py to check whether intersection_update() works correctly when it gets one or multiple operands. I also add a test for __iand__ as a control. My intersection_update() tests fail when run on the original implementation, and pass when run on my version. Also, in the tests, I add NUM_ITERS and multiply self.SET_SIZE to increase a chance of a race condition occurring.

Testing

• make patchcheck
• ./python.exe -E -Wd -m test -v test_set test_free_threading.test_set

AI Disclosure: I use Codex to help prepare my solution and tests. I have verified them.

• Issue: set.intersection_update() can lose concurrent updates in free-threaded builds #158600

[bedevere-app]

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

[hetaozdh]

I don't think the extra copy is necessary here. Instead of reusing set_intersection_multi_impl we can reuse the single-operand set_intersection_update under Py_BEGIN_CRITICAL_SECTION2(so, other). Do you think following the doing of set_difference_update_impl() is a good idea since it does not have the same problem?

for (i = 0; i < others_length; i++) {
    PyObject *other = others[i];
    PyObject *result;

    Py_BEGIN_CRITICAL_SECTION2(so, other);
    result = set_intersection_update(so, other);
    Py_END_CRITICAL_SECTION2();
    if (result == NULL) {
        return NULL;
    }
    Py_DECREF(result);
}
Py_RETURN_NONE;

It can save the O(len(so)) copy on calls.

One small thing to keep in mind is that if an error occurs, the earlier operands have already been applied. This is consistent with set.difference_update(), so I don't think it is really a big problem.

[CaQtiml]

I did not realize that difference_update() allows a partial update on failure. Thank you for pointing this out! However, a reason I propose this approach is the partial update problem.

The current implementation of intersection_update() does not partially update the target on failure, so I want to keep this behavior. I agree that your solution is faster, but changing the method's behavior could affect existing code, including code running with the GIL enabled.

[hetaozdh]

What do you think of the idea that use a for loop to calculate the final result and do only one swap. I think it keeps the same behavior and also saves the copy. :-)

[CaQtiml]

This is a nice and good idea! At first, I tried my best to keep using set_intersection_multi_impl() to avoid code duplication, but I think the duplication is acceptable here because it avoids the overhead from a copy.