melib: return Result<_> from operation()

Envelope might have been deleted before main thread requests an
operation, which is a race condition.
async
Manos Pitsidianakis 2020-06-23 17:21:50 +03:00
parent d827ea1001
commit efb06be09b
Signed by: Manos Pitsidianakis
GPG Key ID: 73627C2F690DF710
14 changed files with 153 additions and 81 deletions

View File

@ -297,7 +297,7 @@ pub trait MailBackend: ::std::fmt::Debug + Send + Sync {
work_context: WorkContext,
) -> Result<std::thread::ThreadId>;
fn mailboxes(&self) -> Result<HashMap<MailboxHash, Mailbox>>;
fn operation(&self, hash: EnvelopeHash) -> Box<dyn BackendOp>;
fn operation(&self, hash: EnvelopeHash) -> Result<Box<dyn BackendOp>>;
fn save(&self, bytes: &[u8], mailbox_hash: MailboxHash, flags: Option<Flag>) -> Result<()>;
fn delete(&self, _env_hash: EnvelopeHash, _mailbox_hash: MailboxHash) -> Result<()> {

View File

@ -625,9 +625,17 @@ impl MailBackend for ImapType {
.collect())
}
fn operation(&self, hash: EnvelopeHash) -> Box<dyn BackendOp> {
let (uid, mailbox_hash) = self.uid_store.hash_index.lock().unwrap()[&hash];
Box::new(ImapOp::new(
fn operation(&self, hash: EnvelopeHash) -> Result<Box<dyn BackendOp>> {
let (uid, mailbox_hash) = if let Some(v) =
self.uid_store.hash_index.lock().unwrap().get(&hash)
{
*v
} else {
return Err(MeliError::new(
"Message not found in local cache, it might have been deleted before you requested it."
));
};
Ok(Box::new(ImapOp::new(
uid,
self.uid_store.mailboxes.read().unwrap()[&mailbox_hash]
.imap_path()
@ -635,7 +643,7 @@ impl MailBackend for ImapType {
mailbox_hash,
self.connection.clone(),
self.uid_store.clone(),
))
)))
}
fn save(&self, bytes: &[u8], mailbox_hash: MailboxHash, flags: Option<Flag>) -> Result<()> {

View File

@ -254,12 +254,12 @@ impl MailBackend for JmapType {
.collect())
}
fn operation(&self, hash: EnvelopeHash) -> Box<dyn BackendOp> {
Box::new(JmapOp::new(
fn operation(&self, hash: EnvelopeHash) -> Result<Box<dyn BackendOp>> {
Ok(Box::new(JmapOp::new(
hash,
self.connection.clone(),
self.store.clone(),
))
)))
}
fn save(&self, _bytes: &[u8], _mailbox_hash: MailboxHash, _flags: Option<Flag>) -> Result<()> {

View File

@ -648,12 +648,12 @@ impl MailBackend for MaildirType {
Ok(handle.thread().id())
}
fn operation(&self, hash: EnvelopeHash) -> Box<dyn BackendOp> {
Box::new(MaildirOp::new(
fn operation(&self, hash: EnvelopeHash) -> Result<Box<dyn BackendOp>> {
Ok(Box::new(MaildirOp::new(
hash,
self.hash_indexes.clone(),
self.mailbox_index.lock().unwrap()[&hash],
))
)))
}
fn save(&self, bytes: &[u8], mailbox_hash: MailboxHash, flags: Option<Flag>) -> Result<()> {

View File

@ -894,7 +894,8 @@ impl MailBackend for MboxType {
.map(|(h, f)| (*h, f.clone() as Mailbox))
.collect())
}
fn operation(&self, env_hash: EnvelopeHash) -> Box<dyn BackendOp> {
fn operation(&self, env_hash: EnvelopeHash) -> Result<Box<dyn BackendOp>> {
let mailbox_hash = self.mailbox_index.lock().unwrap()[&env_hash];
let mailboxes_lck = self.mailboxes.lock().unwrap();
let (offset, length) = {
@ -902,12 +903,12 @@ impl MailBackend for MboxType {
index[&env_hash]
};
let mailbox_path = mailboxes_lck[&mailbox_hash].fs_path.clone();
Box::new(MboxOp::new(
Ok(Box::new(MboxOp::new(
env_hash,
mailbox_path.as_path(),
offset,
length,
))
)))
}
fn save(&self, _bytes: &[u8], _mailbox_hash: MailboxHash, _flags: Option<Flag>) -> Result<()> {

View File

@ -599,17 +599,20 @@ impl MailBackend for NotmuchDb {
.map(|(k, f)| (*k, BackendMailbox::clone(f)))
.collect())
}
fn operation(&self, hash: EnvelopeHash) -> Box<dyn BackendOp> {
Box::new(NotmuchOp {
database: Arc::new(
Self::new_connection(self.path.as_path(), self.lib.clone(), true).unwrap(),
),
fn operation(&self, hash: EnvelopeHash) -> Result<Box<dyn BackendOp>> {
Ok(Box::new(NotmuchOp {
database: Arc::new(Self::new_connection(
self.path.as_path(),
self.lib.clone(),
true,
)?),
lib: self.lib.clone(),
hash,
index: self.index.clone(),
bytes: None,
tag_index: self.tag_index.clone(),
})
}))
}
fn save(&self, bytes: &[u8], _mailbox_hash: MailboxHash, flags: Option<Flag>) -> Result<()> {
@ -621,7 +624,7 @@ impl MailBackend for NotmuchDb {
crate::backends::MaildirType::save_to_mailbox(path, bytes, flags)
}
fn as_any(&self) -> &dyn ::std::any::Any {
fn as_any(&self) -> &dyn::std::any::Any {
self
}

View File

@ -173,7 +173,7 @@ impl Composer {
pub fn edit(account_pos: usize, h: EnvelopeHash, context: &Context) -> Result<Self> {
let mut ret = Composer::default();
let op = context.accounts[account_pos].operation(h);
let op = context.accounts[account_pos].operation(h)?;
let envelope: EnvelopeRef = context.accounts[account_pos].collection.get_env(h);
ret.draft = Draft::edit(&envelope, op)?;
@ -185,7 +185,7 @@ impl Composer {
pub fn with_context(
coordinates: (usize, MailboxHash),
msg: EnvelopeHash,
context: &Context,
context: &mut Context,
) -> Self {
let account = &context.accounts[coordinates.0];
let mut ret = Composer::default();
@ -230,10 +230,19 @@ impl Composer {
}
}
let mut op = account.operation(msg);
let parent_bytes = op.as_bytes();
ret.draft = Draft::new_reply(&parent_message, parent_bytes.unwrap());
match account.operation(msg) {
Err(err) => {
context.replies.push_back(UIEvent::Notification(
None,
err.to_string(),
Some(NotificationType::ERROR),
));
}
Ok(mut op) => {
let parent_bytes = op.as_bytes();
ret.draft = Draft::new_reply(&parent_message, parent_bytes.unwrap());
}
}
let subject = parent_message.subject();
ret.draft.headers_mut().insert(
"Subject".into(),

View File

@ -156,7 +156,16 @@ pub trait MailListingTrait: ListingTrait {
}
for env_hash in envs_to_set {
let account = &mut context.accounts[self.coordinates().0];
let mut op = account.operation(env_hash);
let mut op =
match account.operation(env_hash) {
Ok(op) => op,
Err(err) => {
context.replies.push_back(UIEvent::StatusEvent(
StatusEvent::DisplayMessage(err.to_string()),
));
continue;
}
};
let mut envelope: EnvelopeRefMut = account.collection.get_env_mut(env_hash);
match a {
ListingAction::SetSeen => {
@ -1462,7 +1471,7 @@ impl Listing {
context
.replies
.push_back(UIEvent::StatusEvent(StatusEvent::UpdateStatus(
self.get_status(context)
self.get_status(context),
)));
self.menu_cursor_pos = self.cursor_pos;
}

View File

@ -215,15 +215,25 @@ impl MailListingTrait for CompactListing {
SmallVec::new(),
);
for thread in items {
'items_for_loop: for thread in items {
let thread_node = &threads.thread_nodes()[&threads.thread_ref(thread).root()];
let root_env_hash = thread_node.message().unwrap_or_else(|| {
let root_env_hash = if let Some(h) = thread_node.message().or_else(|| {
if thread_node.children().is_empty() {
return None;
}
let mut iter_ptr = thread_node.children()[0];
while threads.thread_nodes()[&iter_ptr].message().is_none() {
if threads.thread_nodes()[&iter_ptr].children().is_empty() {
return None;
}
iter_ptr = threads.thread_nodes()[&iter_ptr].children()[0];
}
threads.thread_nodes()[&iter_ptr].message().unwrap()
});
threads.thread_nodes()[&iter_ptr].message()
}) {
h
} else {
continue 'items_for_loop;
};
if !context.accounts[self.cursor_pos.0].contains_key(root_env_hash) {
debug!("key = {}", root_env_hash);
debug!(

View File

@ -997,29 +997,23 @@ impl PlainListing {
fn perform_action(&mut self, context: &mut Context, env_hash: EnvelopeHash, a: &ListingAction) {
let account = &mut context.accounts[self.cursor_pos.0];
let hash = account.collection.get_env(env_hash).hash();
let op = account.operation(hash);
let mut envelope: EnvelopeRefMut = account.collection.get_env_mut(env_hash);
match a {
ListingAction::SetSeen => {
if let Err(e) = envelope.set_seen(op) {
context
.replies
.push_back(UIEvent::StatusEvent(StatusEvent::DisplayMessage(
e.to_string(),
)));
if let Err(e) = account.operation(hash).and_then(|op| {
let mut envelope: EnvelopeRefMut = account.collection.get_env_mut(env_hash);
match a {
ListingAction::SetSeen => envelope.set_seen(op),
ListingAction::SetUnseen => envelope.set_unseen(op),
ListingAction::Delete => {
/* do nothing */
Ok(())
}
_ => unreachable!(),
}
ListingAction::SetUnseen => {
if let Err(e) = envelope.set_unseen(op) {
context
.replies
.push_back(UIEvent::StatusEvent(StatusEvent::DisplayMessage(
e.to_string(),
)));
}
}
ListingAction::Delete => { /* do nothing */ }
_ => unreachable!(),
}) {
context
.replies
.push_back(UIEvent::StatusEvent(StatusEvent::DisplayMessage(
e.to_string(),
)));
}
self.row_updates.push(env_hash);
}

View File

@ -314,10 +314,11 @@ impl Component for MailView {
(envelope.hash(), envelope.is_seen())
};
if !is_seen {
let op = account.operation(hash);
let mut envelope: EnvelopeRefMut =
account.collection.get_env_mut(self.coordinates.2);
if let Err(e) = envelope.set_seen(op) {
if let Err(e) = account.operation(hash).and_then(|op| {
let mut envelope: EnvelopeRefMut =
account.collection.get_env_mut(self.coordinates.2);
envelope.set_seen(op)
}) {
context
.replies
.push_back(UIEvent::StatusEvent(StatusEvent::DisplayMessage(format!(
@ -553,8 +554,10 @@ impl Component for MailView {
let body = {
let account = &mut context.accounts[self.coordinates.0];
let envelope: EnvelopeRef = account.collection.get_env(self.coordinates.2);
let op = account.operation(envelope.hash());
match envelope.body(op) {
match account
.operation(envelope.hash())
.and_then(|op| envelope.body(op))
{
Ok(body) => body,
Err(e) => {
clear_area(
@ -630,7 +633,17 @@ impl Component for MailView {
let text = {
let account = &context.accounts[self.coordinates.0];
let envelope: EnvelopeRef = account.collection.get_env(self.coordinates.2);
let mut op = account.operation(envelope.hash());
let mut op = match account.operation(envelope.hash()) {
Ok(op) => op,
Err(err) => {
context.replies.push_back(UIEvent::Notification(
Some("Failed to open e-mail".to_string()),
err.to_string(),
Some(NotificationType::ERROR),
));
return;
}
};
if source == Source::Raw {
op.as_bytes()
.map(|v| String::from_utf8_lossy(v).into_owned())
@ -924,9 +937,10 @@ impl Component for MailView {
{
let account = &mut context.accounts[self.coordinates.0];
let envelope: EnvelopeRef = account.collection.get_env(self.coordinates.2);
let op = account.operation(envelope.hash());
let attachments = match envelope.body(op) {
let attachments = match account
.operation(envelope.hash())
.and_then(|op| envelope.body(op))
{
Ok(body) => body.attachments(),
Err(e) => {
context.replies.push_back(UIEvent::Notification(
@ -983,9 +997,11 @@ impl Component for MailView {
{
let account = &mut context.accounts[self.coordinates.0];
let envelope: EnvelopeRef = account.collection.get_env(self.coordinates.2);
let op = account.operation(envelope.hash());
let attachments = match envelope.body(op) {
let attachments = match account
.operation(envelope.hash())
.and_then(|op| envelope.body(op))
{
Ok(body) => body.attachments(),
Err(e) => {
context.replies.push_back(UIEvent::Notification(
@ -1163,8 +1179,10 @@ impl Component for MailView {
let account = &mut context.accounts[self.coordinates.0];
let envelope: EnvelopeRef = account.collection.get_env(self.coordinates.2);
let finder = LinkFinder::new();
let op = account.operation(envelope.hash());
let t = match envelope.body(op) {
let t = match account
.operation(envelope.hash())
.and_then(|op| envelope.body(op))
{
Ok(body) => body.text().to_string(),
Err(e) => {
context.replies.push_back(UIEvent::Notification(
@ -1233,7 +1251,25 @@ impl Component for MailView {
use std::io::Write;
let account = &mut context.accounts[self.coordinates.0];
let envelope: EnvelopeRef = account.collection.get_env(self.coordinates.2);
let mut op = account.operation(envelope.hash());
let mut op = match account.operation(envelope.hash()) {
Ok(b) => b,
Err(err) => {
context.replies.push_back(UIEvent::Notification(
Some("Failed to open e-mail".to_string()),
err.to_string(),
Some(NotificationType::ERROR),
));
log(
format!(
"Failed to open envelope {}: {}",
envelope.message_id_display(),
err.to_string()
),
ERROR,
);
return true;
}
};
if a_i == 0 {
let mut path = std::path::Path::new(path).to_path_buf();

View File

@ -963,13 +963,13 @@ impl Account {
pub fn contains_key(&self, h: EnvelopeHash) -> bool {
self.collection.contains_key(&h)
}
pub fn operation(&self, h: EnvelopeHash) -> Box<dyn BackendOp> {
let operation = self.backend.read().unwrap().operation(h);
if self.settings.account.read_only() {
pub fn operation(&self, h: EnvelopeHash) -> Result<Box<dyn BackendOp>> {
let operation = self.backend.read().unwrap().operation(h)?;
Ok(if self.settings.account.read_only() {
ReadOnlyOp::new(operation)
} else {
operation
}
})
}
pub fn thread(&self, h: ThreadNodeHash, f: MailboxHash) -> &ThreadNode {
@ -1209,7 +1209,11 @@ impl Account {
ret.push(env_hash);
continue;
}
let op = self.operation(env_hash);
let op = if let Ok(op) = self.operation(env_hash) {
op
} else {
continue;
};
let body = envelope.body(op)?;
let decoded = decode_rec(&body, None);
let body_text = String::from_utf8_lossy(&decoded);

View File

@ -204,13 +204,13 @@ impl MailBackend for PluginBackend {
Ok(ret)
}
fn operation(&self, hash: EnvelopeHash) -> Box<dyn BackendOp> {
Box::new(PluginOp {
fn operation(&self, hash: EnvelopeHash) -> Result<Box<dyn BackendOp>> {
Ok(Box::new(PluginOp {
hash,
channel: self.channel.clone(),
tag_index: self.tag_index.clone(),
bytes: None,
})
}))
}
fn save(&self, _bytes: &[u8], _mailbox_hash: MailboxHash, _flags: Option<Flag>) -> Result<()> {

View File

@ -147,8 +147,7 @@ pub fn insert(
let conn = melib_sqlite3::open_db(db_path)?;
let backend_lck = backend.read().unwrap();
let op = backend_lck.operation(envelope.hash());
let body = match envelope.body(op) {
let body = match backend_lck.operation(envelope.hash()).and_then(|op| envelope.body(op)) {
Ok(body) => body.text(),
Err(err) => {
debug!(
@ -332,8 +331,7 @@ pub fn index(context: &mut crate::state::Context, account_name: &str) -> Result<
let backend_lck = backend_mutex.read().unwrap();
for env_hash in chunk {
if let Some(e) = envelopes_lck.get(&env_hash) {
let op = backend_lck.operation(e.hash());
let body = match e.body(op) {
let body = match backend_lck.operation(e.hash()).and_then(|op| e.body(op)) {
Ok(body) => body.text(),
Err(err) => {
debug!("{}",