Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement _thread.RLock #1922

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

slozier
Copy link
Contributor

@slozier slozier commented Feb 25, 2025

Implement _thread.RLock and enable some test_threading tests.

I got bit by functools.lru_cache which uses a dummy lock if _thread.RLock is not available. 😢

@slozier
Copy link
Contributor Author

slozier commented Feb 26, 2025

Probably not related to the PR, but got this failure during CI on Ubuntu:

2025-02-26T00:44:34.1619783Z [net462 cpython]  ERROR: test_fcntl_64_bit (__main__.TestFcntl)
2025-02-26T00:44:34.1620469Z [net462 cpython]  ----------------------------------------------------------------------
2025-02-26T00:44:34.1625679Z [net462 cpython]  Traceback (most recent call last):
2025-02-26T00:44:34.1626165Z [net462 cpython]    File "/home/vsts/work/1/s/src/core/IronPython.StdLib/lib/test/test_fcntl.py", line 121, in test_fcntl_64_bit
2025-02-26T00:44:34.1629707Z [net462 cpython]      fd = os.open(os.path.dirname(os.path.abspath(TESTFN)), os.O_RDONLY)
2025-02-26T00:44:34.1630169Z [net462 cpython]  PermissionError: [Errno 13] Permission denied: '/home/vsts/work/1/s/src/core/IronPython.StdLib/lib/test'

Copy link
Member

@BCSharp BCSharp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@BCSharp
Copy link
Member

BCSharp commented Feb 26, 2025

Probably not related to the PR, but got this failure during CI on Ubuntu

Were there any other tests running in parallel? Looks like not-parallel-safe issue.

@slozier
Copy link
Contributor Author

slozier commented Feb 26, 2025

Probably not related to the PR, but got this failure during CI on Ubuntu

Were there any other tests running in parallel? Looks like not-parallel-safe issue.

I also saw it on github CI (see snippet below, not sure if you have access, but here are the raw logs) where each framework runs one after the other so there should be no parallel issues (although cpython and ironpython stuff could be running at the same time I guess).

Also weird is that the cpython job "succeeded" so we might have some testing issues...

2025-02-25T22:17:01.2597886Z [net462 cpython]  ======================================================================
2025-02-25T22:17:01.2598561Z [net462 cpython]  ERROR: test_fcntl_64_bit (__main__.TestFcntl)
2025-02-25T22:17:01.2599287Z [net462 cpython]  ----------------------------------------------------------------------
2025-02-25T22:17:01.2599943Z [net462 cpython]  Traceback (most recent call last):
2025-02-25T22:17:01.2601216Z [net462 cpython]    File "/home/runner/work/ironpython3/ironpython3/src/core/IronPython.StdLib/lib/test/test_fcntl.py", line 121, in test_fcntl_64_bit
2025-02-25T22:17:01.2602510Z [net462 cpython]      fd = os.open(os.path.dirname(os.path.abspath(TESTFN)), os.O_RDONLY)
2025-02-25T22:17:01.2603825Z [net462 cpython]  PermissionError: [Errno 13] Permission denied: '/home/runner/work/ironpython3/ironpython3/src/core/IronPython.StdLib/lib/test'
2025-02-25T22:17:01.2629641Z [net462 cpython]  ----------------------------------------------------------------------
2025-02-25T22:17:01.2630274Z [net462 cpython]  Ran 7 tests in 0.051s
2025-02-25T22:17:01.2630768Z [net462 cpython]  FAILED (errors=1, skipped=2)
...
2025-02-25T22:18:30.6930393Z [net462 cpython] succeeded
2025-02-25T22:18:30.6930654Z 1 jobs running: [net462 ironpython]
2025-02-25T22:18:30.6930902Z 0 jobs pending: 

@BCSharp
Copy link
Member

BCSharp commented Feb 26, 2025

I also saw it on github CI (see snippet below, not sure if you have access, but here are the raw logs) where each framework runs one after the other so there should be no parallel issues (although cpython and ironpython stuff could be running at the same time I guess).

I don't have access to those logs but I was able to see the failures in one of your failed runs in this PR and have reproduced it locally. It only happens on Mono. It is caused by the implementation of os.open, which on Mono, does not support opening directories. Recently, when working on the support of genuine file descriptors, I have migrated the .NET implementation of open to use a direct syscall, which can open directories. I have left Mono to do it the old way, not to introduce regression. That is, on Mono, FileStream is still being used for opening, mainly due to limitations in Mono's MemoryMappedFile.

The test used to pass without problems because it is being skipped if fcntl.F_NOTIFY or fcntl.DN_MULTISHOT are not defined, and these were added only very recently (the test is also skipped on macOS and on Linux/ARM). I don't want to paddle upstream with Mono, so I am inclined to leave it where it is, and simply disable the test on Mono. But instead of disabling it in the .ini file completely (the other tests in test_fcntl are passing and doing some useful tests) or create yet another fine-grained ..._stdlib test ovveride (extra overhead for IronPython tests), how about we simply hide the constant F_NOTIFY and all theDN_ constants in fcntl on Mono builds? This will cause the test to be skipped, and, since you can't get an open directory descriptor on Mono (by using the regular Python API at least), F_NOTIFY on Mono is pretty much useless anyway.

@slozier
Copy link
Contributor Author

slozier commented Feb 27, 2025

I'm fine with hiding the constants on Mono. I was going to ask how we'd accomplish that but I guess a simple #if NET (or does it need to be NETCOREAPP) would do the trick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants