fix deadlock between require_lock and package callbacks

[JeffBezanson]

Fix the deadlock described in #43339 (comment)

Also fix one place where we were acquiring require_lock twice (which is ok since it's reentrant, but not preferred).

[tkf]

Also fix one place where we were acquiring require_lock twice (which is ok since it's reentrant, but not preferred).

Isn't the strategy you've implemented in this PR actually relies on this? IIUC, the intention of run_package_callbacks is to release the require_lock "completely" so that require_lock is not acquired by the time the callbacks are called.

[vtjnash]

we perhaps should have an assert that the lock state is unlocked, so that it cannot hang?

[JeffBezanson]

Isn't the strategy you've implemented in this PR actually relies on this?

I don't understand --- if it's locked twice, the unlock is not sufficient, so we need it to be locked only once at a time.

[tkf]

By "this", I mean the patch introducing _require_prelocked. I should've avoided using the word "this". I meant that, since run_package_callbacks requires to turn require_lock to unlocked state, we can't lock require_lock multiple times in require methods. Without _require_prelocked, the call chain like require(into::Module, mod::Symbol) -> require(uuidkey::PkgId) -> run_package_callbacks would have called the callbacks while require_lock is still being locked.

To put it in another way, your comment in the OP sounded like the change introducing _require_prelocked was optional. I thought it was a requirement.