Enforce no-move rule of ReentrantMutex using Pin and fix UB in stdio by m-ou-se...

m-ou-se commented on Oct 11 •

A sys_common::ReentrantMutex may not be moved after initializing it with .init(). This was not enforced, but only stated as a requirement in the comments on the unsafe functions. This change enforces this no-moving rule using Pin, by changing &self to a Pin in the init() and lock() functions.

This uncovered a bug I introduced in #77154: stdio.rs (the only user of ReentrantMutex) called init() on its ReentrantMutexes while constructing them in the intializer of SyncOnceCell::get_or_init, which would move them afterwards. Interestingly, the ReentrantMutex unit tests already had the same bug, so this invalid usage has been tested on all (CI-tested) platforms for a long time. Apparently this doesn't break badly on any of the major platforms, but it does break the rules.*

To be able to keep using SyncOnceCell, this adds a SyncOnceCell::get_or_init_pin function, which makes it possible to work with pinned values inside a (pinned) SyncOnceCell. Whether this function should be public or not and what its exact behaviour and interface should be if it would be public is something I'd like to leave for a separate issue or PR. In this PR, this function is internal-only and marked with pub(crate).

* Note: That bug is now included in 1.48, while this patch can only make it to 1.49 1.50. We should consider the implications of 1.48 shipping with a wrong usage of pthread_mutex_t / CRITICAL_SECTION / .. which technically invokes UB according to their specification. The risk is very low, considering the objects are not 'used' (locked) before the move, and the ReentrantMutex unit tests have verified this works fine in practice.

Edit: This has been backported and included in 1.48. And soon 1.49 too.

In future changes, I want to push this usage of Pin further inside sys instead of only sys_common, and apply it to all 'unmovable' objects there (Mutex, Condvar, RwLock). Also, while sys_common's mutexes and condvars are already taken care of by #77147 and #77648, its RwLock should still be made movable or get pinned.