Cleanup rwobject.c file object methods

[Starbuck5]

• Simplifies the code by removing WITH_THREAD / not WITH_THREAD conditional compilation
• Removes path for file-like objects that are actually files
• Removes unused data ("file") inside pgRwHelper

I've tried before to remove WITH_THREAD blocks and got a lot of skepticism, so I've done my homework this time. See CPython python/cpython#75551 "Remove support for threads-less builds," where WITH_THREAD becomes always defined (Python 3.7). See also pyport.h where it is still hardcoded as defined, and the region below references emscripten's lack of threading support, so it's not like this code hasn't taken that into account yet. Lastly, lets say we did build on a platform without threads... what's the harm in calling PyGILState_Ensure and PyGILState_Release? They're both in the stable ABI, it's not like they will be missing-- they might become no-ops but that's fine.

Secondly, my removal of the fileno path. The io protocol can only provide a fileno if it actually comes from a file, see https://docs.python.org/3/library/io.html#io.IOBase.fileno. I believe it would be very uncommon for a user to pass an io object into a pygame loader after loading from a file. What I've seen is str/pathlib path input, which creates a basic SDL_RWop, and io.BytesIO-style (memory) input which creates one of our custom RWops. io.BytesIO objects don't come from files, so they don't have a fileno. Before this PR every time they had to do anything they'd have to check if they had a fileno. If a user happens to pass in an io object that actually does come from a file, the code using the standard io APIs will work fine on it without a specialized path for that case.

Also, per the docs for PyObject_AsFileDescriptor (at https://docs.python.org/3/c-api/file.html), it is an emulation of Python 2 APIs and third party code should use io instead.

Some example code showing how uses of io can intersect with pygame-ce:

import pygame
import io

pygame.init()

# Load into BytesIO somehow (through a file this time but could come from anywhere)
with open(pygame.font.match_font("Arial"), "rb") as f:
    obj = io.BytesIO(f.read())

# print(obj.fileno()) # obj doesn't have a fileno, raises exception
font = pygame.Font(obj, 20)
print(font)

# f is an io.BufferedReader
with open(pygame.font.match_font("Arial"), "rb") as f:
    print(f.fileno()) # f has a fileno
    font = pygame.Font(f, 20)
    print(font)

[ankith26]

I'm in for removing WITH_THREAD. But I don't think we need to remove fileno logic, simply because it is intended as an optimisation (as that codepath does not need GIL) and it doesn't add much code complexity.

[Starbuck5]

Optimizations are all well and good, but an optimization for who? I don't think I've ever seen anyone open a file and pass it into pygame-ce in the way required to get this to work, in years of examining people's codes on discord and on reddit.

Keep in mind removing the branching and ref counting required for this questionable optimization is a micro-optimization for the far more common case of using a BytesIO. And it makes the code simpler.

[ankith26]

I looked at the blame for when this code was added, and traced it back to pygame/pygame#551

Back when it was added it was a bugfix and not an optimization. Though now, I just tested your PR and it works with the testcase that used to deadlock the music playback before this fix.

[ankith26]

Okay, these changes LGTM, thanks!

I guess it would still be nice to add a few unitests that explicitly test the python-with-fileno kind of objects while simultaneously running multiple instances of those to make sure the deadlocking issue doesn't return.

Also there are more usages of WITH_THREAD around the codebase that could be removed, this could also happen in a later PR

[Starbuck5]

Thanks for looking into the history of this, I had assumed it was primordial and wouldn't have a PR attached.

I really was not thinking about the GIL at all while making this PR.

The old PR that added this seems like a sidestep of the issue. It avoids the deadlock only file objects from files, but what about all other file objects? Nobody has reported any deadlocks with BytesIO, and this brings (very uncommon) file-objects-from-files into the same level as BytesIO, so it should be fine.

[Starbuck5]

FYI I was looking into these methods because in SDL3 the rwops signatures changes, so I wanted to simplify them before needing to write in SDL3-compat stuff.

[MyreMylar]

OK, LGTM 👍