Skip to content

Commit

Permalink
Make Resettable lazy again (pantsbuild#7222)
Browse files Browse the repository at this point in the history
### Problem

In the context of pantsbuild#6817, there is a logging issue that manifests when the daemon forks. In particular, in `fork_context`, both the daemon and the client reset some services that implement `Resettable`. Some of those services log at startup, when the client hasn't had time to reconfigure its logging to stderr, and therefore all these startup logs are intermingled in the `pantsd.log` file.

### Solution

If we make `Resettable` lazy again, since we are ensured to only enter `fork_context` under a lock, the logging can only happen when the client has had time to configure its loggers.

### Result

`Resettable` is now lazy. `Resettable::get()` is now implemented in terms of `Resettable::with`.
  • Loading branch information
blorente authored and Stu Hood committed Feb 6, 2019
1 parent f281642 commit 224c2a0
Showing 1 changed file with 23 additions and 15 deletions.
38 changes: 23 additions & 15 deletions src/rust/engine/resettable/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,33 @@ where
T: Send + Sync,
{
pub fn new<F: Fn() -> T + 'static>(make: F) -> Resettable<T> {
let val = (make)();
Resettable {
val: Arc::new(RwLock::new(Some(val))),
val: Arc::new(RwLock::new(None)),
make: Arc::new(make),
}
}

///
/// Execute f with the value in the Resettable.
/// May lazily initialize the value in the Resettable.
///
/// TODO Explore the use of parking_lot::RWLock::upgradable_read
/// to avoid reacquiring the lock for initialization.
/// This can be used if we are sure that a deadlock won't happen
/// when two readers are trying to upgrade at the same time.
///
pub fn with<O, F: FnOnce(&T) -> O>(&self, f: F) -> O {
let val_opt = self.val.read();
let val = val_opt
.as_ref()
.unwrap_or_else(|| panic!("A Resettable value cannot be used while it is shutdown."));
f(val)
{
let val_opt = self.val.read();
if let Some(val) = val_opt.as_ref() {
return f(val);
}
}
let mut val_write_opt = self.val.write();
if val_write_opt.as_ref().is_none() {
*val_write_opt = Some((self.make)())
}
f(val_write_opt.as_ref().unwrap())
}

///
Expand All @@ -89,9 +103,7 @@ where
{
let mut val = self.val.write();
*val = None;
let t = f();
*val = Some((self.make)());
t
f()
}
}

Expand All @@ -106,10 +118,6 @@ where
/// be sure that dropping it will actually deallocate the resource.
///
pub fn get(&self) -> T {
let val_opt = self.val.read();
let val = val_opt
.as_ref()
.unwrap_or_else(|| panic!("A Resettable value cannot be used while it is shutdown."));
val.clone()
self.with(T::clone)
}
}

0 comments on commit 224c2a0

Please sign in to comment.