gh-151707: fix race conditions in FileIO

[KowalskiThomas]

What is this PR?

Fixes #151707.

Note that the PR itself does a bit more than just fix the race condition. It introduces new helper functions around the self->fd to avoid race conditions altogether, in the same fashion as the socketmodule.c file (and updates every relevant usage site to call into those helpers).

cpython/Modules/socketmodule.c

Lines 566 to 592 in 46b5e3e

	static inline void
	set_sock_fd(PySocketSockObject *s, SOCKET_T fd)
	{
	#if SIZEOF_SOCKET_T == SIZEOF_INT
	_Py_atomic_store_int_relaxed((int *)&s->sock_fd, (int)fd);
	#elif SIZEOF_SOCKET_T == SIZEOF_LONG
	_Py_atomic_store_long_relaxed((long *)&s->sock_fd, (long)fd);
	#elif SIZEOF_SOCKET_T == SIZEOF_LONG_LONG
	_Py_atomic_store_llong_relaxed((long long *)&s->sock_fd, (long long)fd);
	#else
	#error "Unsupported SIZEOF_SOCKET_T"
	#endif
	}
	static inline SOCKET_T
	get_sock_fd(PySocketSockObject *s)
	{
	#if SIZEOF_SOCKET_T == SIZEOF_INT
	return (SOCKET_T)_Py_atomic_load_int_relaxed((int *)&s->sock_fd);
	#elif SIZEOF_SOCKET_T == SIZEOF_LONG
	return (SOCKET_T)_Py_atomic_load_long_relaxed((long *)&s->sock_fd);
	#elif SIZEOF_SOCKET_T == SIZEOF_LONG_LONG
	return (SOCKET_T)_Py_atomic_load_llong_relaxed((long long *)&s->sock_fd);
	#else
	#error "Unsupported SIZEOF_SOCKET_T"
	#endif
	}

• Issue: Race condition in FileIO under free-threading #151707

[vstinner]

I'm not sure that it's the right approach. While the change makes sure that int fd integer is not modified in parallel, it doesn't ensure that the file descriptor is not closed before/while we are using it.

For example, FileIO.isatty() gets int fd, check that it's >= 0, and then call isatty().

If another thread calls close(fd) in parallel, after >= 0 check and before isatty() call, isatty() fails with EBADF.

It's even worse if the file descriptor is recycled for a new unrelated file.

Maybe it would be safer to the whole method in a critical section. It would prevent calling FileIO.close() in parallel: the call would be serialized.

@cmaloney: What do you think?

[vstinner]

Oh, the TSan tests failed. Example:

WARNING: ThreadSanitizer: data race (pid=20851)
  Write of size 8 at 0x72b000000060 by thread T9189:
    #0 close <null> (python+0x1044c7) (BuildId: ac94b2b86fdb18d0b904ed36fb1fd97b68fe5a64)
    #1 internal_close /home/runner/work/cpython/cpython/./Modules/_io/fileio.c:155:15 (python+0x731b66) (BuildId: ac94b2b86fdb18d0b904ed36fb1fd97b68fe5a64)
    #2 _io_FileIO_close_impl /home/runner/work/cpython/cpython/./Modules/_io/fileio.c:210:10 (python+0x730fbb) (BuildId: ac94b2b86fdb18d0b904ed36fb1fd97b68fe5a64)
    #3 _io_FileIO_close /home/runner/work/cpython/cpython/./Modules/_io/clinic/fileio.c.h:34:12 (python+0x730fbb)
...

[KowalskiThomas]

I'm not sure that it's the right approach.

Oh, the TSan tests failed.

I guess we have an answer 😁