Add is_locked_serially() to check if we are in a #[serial] context

[pgerber]

trafficstars

I've recently had this code added to rustls-tls-native:

/// Cargo, at least sometimes, sets SSL_CERT_FILE and SSL_CERT_DIR internally,
/// it uses OpenSSL. So, always unset both at the beginning of a test even if
/// the test doesn't use either.
///
/// # Safety
///
/// This is only safe if used together with `#[serial]` because calling
/// `[env::remove_var()]` is unsafe if another thread is running.
///
/// Note that `env::remove_var()` is scheduled to become unsafe in Rust
/// Edition 2024.
pub(crate) unsafe fn clear_env() {
    env::remove_var("SSL_CERT_FILE");
    env::remove_var("SSL_CERT_DIR");
}

clear_env() is called from various tests and it's really easy to forget #[serial]. This change will make it possible to add an assert!() to ensure we didn't forget:

pub(crate) unsafe fn clear_env() {
    assert!(serial_test::is_locked_serially(None));

    env::remove_var("SSL_CERT_FILE");
    env::remove_var("SSL_CERT_DIR");
}

[palfrey]

Can you add some tests for this please? Particular cases of interest: behaviour inside a serial lock, behaviour outside of a serial lock, behaviour on a lock where's there's more than one name.

You can run the lock functions directly, like in unlock_on_assert (although minus the std::panic bits obviously).

[pgerber]

Looking at the test results, I guess the problem is that I check if there if the lock with given name is locked (or unlocked) but it could be locked by another test/thread.

[palfrey]

Looking at the test results, I guess the problem is that I check if there if the lock with given name is locked (or unlocked) but it could be locked by another test/thread.

Agreed. OTOH, I don't think you need to have the same names in the different tests to be able to do the checks properly which would solve this. Also, in order to do it with the same names would effectively need another serial_test instance to do this one at a time, without using the existing locks!

[pgerber]

Using the same name was quite intentional. The reason for the pull request, as described above, is to be able to use assert!() to ensure that we are holding a serial lock. If I interpret the test result correctly, it's, however, possible that the assert!() passes because another test/thread is holding the lock. I think my implementation is wrong and the tests are are complaining rightfully.

[pgerber]

This implementation should be better, I think. It's hard to tell as I'm having trouble reproducing the issue reported by CI locally.

One thing I've not looked into yet is how this interacts with async.

[palfrey]

Finally got around to releasing this as part of 3.2.0