so and otherset need to be both locked when being operated on

[AZero13]

Bug report

Bug description:

set_symmetric_difference_update_impl is not locking both when they should be locked. Not sure if this is easy to trigger in practice.

CPython versions tested on:

CPython main branch

Operating systems tested on:

macOS

Linked PRs

• gh-142399: Lock both so and otherset when setting a symmetric difference #142400

[serhiy-storchaka]

What happens if one thread calls a.symmetric_difference_update(b) and other calls b.symmetric_difference_update(a)?

cc @rhettinger

[eendebakpt]

There is indeed an assert statement inside set_symmetric_difference_update_set on both arguments being locked:

cpython/Objects/setobject.c

Lines 1946 to 1951 in 3db7bf2

	static int
	set_symmetric_difference_update_set(PySetObject *so, PySetObject *other)
	{
	_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(so);
	_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(other);

But we can probable relax this to one argument being locked and and the other being uniquely referenced. (this is what seems to be used in the set_symmetric_difference_update_impl).

[ZeroIntensity]

No need, _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED already checks for a refcount of 1:

cpython/Include/internal/pycore_critical_section.h

Lines 55 to 58 in 8620d30

	# define _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op) \
	if (Py_REFCNT(op) != 1) { \
	_PyCriticalSection_AssertHeldObj(_PyObject_CAST(op)); \
	}

[kumaraditya303]

The otherset is local to the thread as it is newly created so no need for locking.