GeezerVison

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

Error: 'frozenset' object has no attribute 'discard'

Legend:

• [+] path exists in CPython
• [x] up-to-date, [ ] outdated

@coderabbitai I have enabled test_issue142594 in this PR which does exactly what you suggested (calling wrapper.detach() inside the closed property). It should prevent regressions for this fix going forward.

#!/bin/bash
# Search for test_issue142594 in the codebase
rg -n "test_issue142594" -A 30

#!/bin/bash
# Search for 142594 in the codebase
rg -n "142594" -C 5

# Also search for test files related to io
fd -e py test.*io

#!/bin/bash
# Search more broadly for test files and the test
fd -t f "test_" | head -20

# Look for any references to the test being skipped or enabled
rg -n "issue142594" -i

# Check common test directories
fd -t d -d 2 "test"

#!/bin/bash
# Search for test files in Lib directory
fd -t f "test_io" Lib/

# Search for the test name in Lib
rg -n "test_issue142594" Lib/ -A 20

# Also check for any recent changes to test files
git diff --name-only origin/main | grep -i test

#!/bin/bash
# Look at the git diff for test_io.py to see what changed
git diff origin/main -- Lib/test/test_io.py | head -100

# Also search the current version of the file
rg -n "test_issue142594" --type py -A 25

@ever0de, excellent! I can see you've removed the @unittest.expectedFailure decorator from test_issue142594 in Lib/test/test_io.py. The test was previously marked as expected failure with the comment:

# TODO: RUSTPYTHON; AttributeError: 'NoneType' object has no attribute 'closed'

This is exactly the issue your fix addresses! By enabling this test, you've ensured that:

1. The reentrant detach() scenario (called via a custom closed property) is properly handled
2. The fix will be protected against future regressions
3. RustPython's behavior now matches CPython for this edge case

This is a great way to validate the fix and maintain test coverage for this subtle reentrancy issue. The test suite will now catch any future changes that might reintroduce the AttributeError on NoneType problem. 🎉

    (\(\
    ( -.-)
    o_(")(")
    [Test coverage achieved!]