matthiasbeyer/imag
1
0
Fork 0
Code Releases Activity

Add flag for _update() whether precense should be modified

Browse source 
This is a bugfix for an very particular issue.

Here's what happens:

If we create() an FileLockEntry and then update() it, we are running
into a problem because update() calls _update() which changes the
precense status of a FileLockEntry.

Because update() is _consuming_, the FileLockEntry gets drop()ed
afterwards. This _again_ causes _update() to be called, but with a new
presence status, which is not true in this moment (as the FileLockEntry
is still borrowed, but we already marked it as present).

This patch is a short-term fix. The real problem is, that our Store
interface is consuming. If the Store interface would be non-consuming,
this issue wouldn't happen, as the drop() call would not happen.

I'm rather sure that this patch will not be reverted in the process of
rewriting the Store interface to be non-consuming. But we never know.
This commit is contained in:
Matthias Beyer 2016-10-07 19:20:34 +02:00
parent 7b11e7dabb
commit 5202c5112a
1 changed files with 7 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
libimagstore/src/store.rs
View file

@ -507,7 +507,7 @@ impl Store {
.map_err_into(SEK::UpdateCallError);
}
if let Err(e) = self._update(&entry) {
if let Err(e) = self._update(&entry, false) {
return Err(e).map_err_into(SEK::UpdateCallError);
}
@ -522,7 +522,7 @@ impl Store {
/// # Assumptions
/// This method assumes that entry is dropped _right after_ the call, hence
/// it is not public.
fn _update<'a>(&'a self, entry: &FileLockEntry<'a>) -> Result<()> {
fn _update<'a>(&'a self, entry: &FileLockEntry<'a>, modify_presence: bool) -> Result<()> {
let mut hsmap = match self.entries.write() {
Err(_) => return Err(SE::new(SEK::LockPoisoned, None)),
Ok(e) => e,
@ -537,7 +537,9 @@ impl Store {
debug!("Writing Entry");
try!(se.write_entry(&entry.entry));
se.status = StoreEntryStatus::Present;
if modify_presence {
se.status = StoreEntryStatus::Present;
}
Ok(())
}
@ -883,7 +885,7 @@ impl<'a> DerefMut for FileLockEntry<'a> {
impl<'a> Drop for FileLockEntry<'a> {
/// This will silently ignore errors, use `Store::update` if you want to catch the errors
fn drop(&mut self) {
let _ = self.store._update(self);
let _ = self.store._update(self, true);
}
}
@ -891,7 +893,7 @@ impl<'a> Drop for FileLockEntry<'a> {
impl<'a> Drop for FileLockEntry<'a> {
/// This will not silently ignore errors but prints the result of the _update() call for testing
fn drop(&mut self) {
println!("Drop Result: {:?}", self.store._update(self));
println!("Drop Result: {:?}", self.store._update(self, true));
}
}