Skip to content

Commit d6511a9

Browse files
committed
Add a few changes suggested by thomcc for the locks.
* Added needed compiler fences to `InitOnce::try_get`. * Change the error in `RawMutex::raw_unlock` to better reflect the cause.
1 parent e4e46a0 commit d6511a9

File tree

1 file changed

+27
-18
lines changed

1 file changed

+27
-18
lines changed

src/sync/locks.rs

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ impl RawMutex {
4040
fn raw_unlock(&self) {
4141
compiler_fence(Ordering::Release);
4242
if !self.0.replace(false) {
43-
already_locked()
43+
panic!("Internal error: Attempt to unlock a `RawMutex` which is not locked.")
4444
}
4545
}
4646

@@ -158,28 +158,37 @@ impl<T> InitOnce<T> {
158158
/// returns `Ok`. If it returns `Err`, it will be called again in the
159159
/// future until an attempt at initialization succeeds.
160160
///
161-
/// Take care when sharing an `InitOnce` object between an IRQ and normal
162-
/// code. If this function is called in an IRQ when it is already currently
163-
/// being initialized by user code, this function will panic.
161+
/// This function will disable interrupts if the contents are not already
162+
/// initialized, which may cause audio skipping and similar issues. This
163+
/// is generally not an issue as that will only happen once during the
164+
/// lifetime of the program.
164165
pub fn try_get<E>(&self, initializer: impl FnOnce() -> Result<T, E>) -> Result<&T, E> {
165166
unsafe {
166167
if self.state.read() != 2 {
167-
// Locks the initializer
168-
if self.state.replace(1) != 0 {
169-
panic!("Attempt to initialize `InitOnce` that is already in initialization.");
170-
}
171-
172-
// Initialize the actual value.
173-
let init = match initializer() {
174-
Ok(v) => v,
175-
Err(e) => {
176-
assert_eq!(self.state.replace(0), 1);
177-
return Err(e);
168+
// We can afford to disable interrupts here, because this code path
169+
// should only be called once for each variable.
170+
with_irqs_disabled(|| -> Result<(), E> {
171+
// Locks the initializer
172+
if self.state.replace(1) != 0 {
173+
panic!("Attempt to initialize `InitOnce` that is already in initialization.");
178174
}
179-
};
180-
ptr::write_volatile((*self.value.get()).as_mut_ptr(), init);
181-
assert_eq!(self.state.replace(2), 1);
175+
176+
// Initialize the actual value.
177+
let init = match initializer() {
178+
Ok(v) => v,
179+
Err(e) => {
180+
assert_eq!(self.state.replace(0), 1);
181+
return Err(e);
182+
}
183+
};
184+
ptr::write_volatile((*self.value.get()).as_mut_ptr(), init);
185+
compiler_fence(Ordering::Release);
186+
assert_eq!(self.state.replace(2), 1);
187+
188+
Ok(())
189+
});
182190
}
191+
compiler_fence(Ordering::Acquire);
183192
Ok(&*(*self.value.get()).as_mut_ptr())
184193
}
185194
}

0 commit comments

Comments
 (0)