Fix potential panics caused by Garbage Collector

[althonos]

As discovered in #854, having a mutable borrow at any point on a PyClass that also implements the Garbage Collection protocol leads to unexpected panics, since the garbage collector can be run at any time, including when our class is already mutably borrowed.

As a solution, the wrapper for the garbage collector now calls PyCell::try_borrow instead of PyCell::borrow, and simply returns if it could not acquire the cell. I'm not sure about the expected behaviour here though, feedback is welcome.

Maintenance

• Run cargo fmt ✅
• Run cargo clippy and check there are no hard errors (There are a bunch of existing warnings; This is also checked by travis) ✅
• If applicable, add an entry in the changelog. ✅
• If applicable, add documentation to all new items and extend the guide.
• If applicable, add tests for all new or fixed functions ✅
• If you changed any python code, run black .. You can install black with pip install black)

[kngwyu]

Really good catch, thank you!

simply returns if it could not acquire the cell.

I think it's OK, though it's really difficult to test. Maybe it's sufficient to check the else branch(=in case try_borrow fails) works correctly?

[althonos]

I'm trying to craft something with a method that blocks while borrowing, and a background thread that calls the GC, but I have no luck currently. I can confirm this test works on the codebase I'm using but since the test case (which comes from the CPython test suite) is very cryptic I can't port it as is.

[davidhewitt]

Looks good to me, thanks 👍

I think that doing nothing if already borrowed is probably fine for tp_traverse - the worst case is that the GC won't detect a cycle, and will do nothing.

I think we might have to look carefully in the future at tp_clear, which I think will only get called once and will need to succeed right away.

[althonos]

Okay, I have a test case where I'm manually invoking the tp_traverse method as-if I were the GC; once while the instance is not borrowed, once when it is mutably borrowed. Without the fix the test panics, with the fix it doesn't. Both branches are covered according to tarpaulin.

[kngwyu]

Awesome 💯 , thank you!

[kngwyu]

Good test 👍