melib/imap: put imap folders in RwLock instead of Mutex

This should prevent lockups if the IMAP conn thread gets blocked
jmap
Manos Pitsidianakis 2019-11-18 12:59:04 +02:00
parent 3c3ee92efb
commit b2cd4f4b7a
Signed by: Manos Pitsidianakis
GPG Key ID: 73627C2F690DF710
2 changed files with 22 additions and 19 deletions

View File

@ -44,7 +44,7 @@ use fnv::{FnvHashMap, FnvHashSet};
use std::collections::hash_map::DefaultHasher; use std::collections::hash_map::DefaultHasher;
use std::hash::Hasher; use std::hash::Hasher;
use std::str::FromStr; use std::str::FromStr;
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex, RwLock};
pub type UID = usize; pub type UID = usize;
pub static SUPPORTED_CAPABILITIES: &'static [&'static str] = &["IDLE"]; pub static SUPPORTED_CAPABILITIES: &'static [&'static str] = &["IDLE"];
@ -100,7 +100,7 @@ pub struct ImapType {
server_conf: ImapServerConf, server_conf: ImapServerConf,
uid_store: Arc<UIDStore>, uid_store: Arc<UIDStore>,
folders: Arc<Mutex<FnvHashMap<FolderHash, ImapFolder>>>, folders: Arc<RwLock<FnvHashMap<FolderHash, ImapFolder>>>,
} }
impl MailBackend for ImapType { impl MailBackend for ImapType {
@ -125,7 +125,7 @@ impl MailBackend for ImapType {
let folder_path = folder.path().to_string(); let folder_path = folder.path().to_string();
let folder_hash = folder.hash(); let folder_hash = folder.hash();
let (permissions, folder_exists) = { let (permissions, folder_exists) = {
let f = &self.folders.lock().unwrap()[&folder_hash]; let f = &self.folders.read().unwrap()[&folder_hash];
(f.permissions.clone(), f.exists.clone()) (f.permissions.clone(), f.exists.clone())
}; };
let connection = self.connection.clone(); let connection = self.connection.clone();
@ -270,13 +270,16 @@ impl MailBackend for ImapType {
} }
fn folders(&self) -> FnvHashMap<FolderHash, Folder> { fn folders(&self) -> FnvHashMap<FolderHash, Folder> {
let mut folders = self.folders.lock().unwrap(); {
if !folders.is_empty() { let folders = self.folders.read().unwrap();
return folders if !folders.is_empty() {
.iter() return folders
.map(|(h, f)| (*h, Box::new(Clone::clone(f)) as Folder)) .iter()
.collect(); .map(|(h, f)| (*h, Box::new(Clone::clone(f)) as Folder))
.collect();
}
} }
let mut folders = self.folders.write().unwrap();
*folders = ImapType::imap_folders(&self.connection); *folders = ImapType::imap_folders(&self.connection);
folders.retain(|_, f| (self.is_subscribed)(f.path())); folders.retain(|_, f| (self.is_subscribed)(f.path()));
let keys = folders.keys().cloned().collect::<FnvHashSet<FolderHash>>(); let keys = folders.keys().cloned().collect::<FnvHashSet<FolderHash>>();
@ -294,7 +297,7 @@ impl MailBackend for ImapType {
let (uid, folder_hash) = self.uid_store.hash_index.lock().unwrap()[&hash]; let (uid, folder_hash) = self.uid_store.hash_index.lock().unwrap()[&hash];
Box::new(ImapOp::new( Box::new(ImapOp::new(
uid, uid,
self.folders.lock().unwrap()[&folder_hash] self.folders.read().unwrap()[&folder_hash]
.path() .path()
.to_string(), .to_string(),
self.connection.clone(), self.connection.clone(),
@ -304,7 +307,7 @@ impl MailBackend for ImapType {
fn save(&self, bytes: &[u8], folder: &str, flags: Option<Flag>) -> Result<()> { fn save(&self, bytes: &[u8], folder: &str, flags: Option<Flag>) -> Result<()> {
let path = { let path = {
let folders = self.folders.lock().unwrap(); let folders = self.folders.read().unwrap();
let f_result = folders.values().find(|v| v.name == folder); let f_result = folders.values().find(|v| v.name == folder);
if f_result if f_result
@ -349,7 +352,7 @@ impl MailBackend for ImapType {
match ( match (
&op, &op,
self.folders self.folders
.lock() .read()
.unwrap() .unwrap()
.values() .values()
.any(|f| f.path == path), .any(|f| f.path == path),
@ -472,7 +475,7 @@ impl ImapType {
server_conf, server_conf,
is_subscribed: Arc::new(IsSubscribedFn(is_subscribed)), is_subscribed: Arc::new(IsSubscribedFn(is_subscribed)),
folders: Arc::new(Mutex::new(Default::default())), folders: Arc::new(RwLock::new(Default::default())),
connection: Arc::new(Mutex::new(connection)), connection: Arc::new(Mutex::new(connection)),
uid_store: Arc::new(UIDStore { uid_store: Arc::new(UIDStore {
uidvalidity: Default::default(), uidvalidity: Default::default(),
@ -575,7 +578,7 @@ impl ImapType {
query: String, query: String,
folder_hash: FolderHash, folder_hash: FolderHash,
) -> Result<crate::structs::StackVec<EnvelopeHash>> { ) -> Result<crate::structs::StackVec<EnvelopeHash>> {
let folders_lck = self.folders.lock()?; let folders_lck = self.folders.read()?;
let mut response = String::with_capacity(8 * 1024); let mut response = String::with_capacity(8 * 1024);
let mut conn = self.connection.lock()?; let mut conn = self.connection.lock()?;
conn.send_command(format!("EXAMINE {}", folders_lck[&folder_hash].path()).as_bytes())?; conn.send_command(format!("EXAMINE {}", folders_lck[&folder_hash].path()).as_bytes())?;

View File

@ -19,7 +19,7 @@
* along with meli. If not, see <http://www.gnu.org/licenses/>. * along with meli. If not, see <http://www.gnu.org/licenses/>.
*/ */
use super::*; use super::*;
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex, RwLock};
/// Arguments for IMAP watching functions /// Arguments for IMAP watching functions
pub struct ImapWatchKit { pub struct ImapWatchKit {
@ -27,7 +27,7 @@ pub struct ImapWatchKit {
pub is_online: Arc<Mutex<bool>>, pub is_online: Arc<Mutex<bool>>,
pub main_conn: Arc<Mutex<ImapConnection>>, pub main_conn: Arc<Mutex<ImapConnection>>,
pub uid_store: Arc<UIDStore>, pub uid_store: Arc<UIDStore>,
pub folders: Arc<Mutex<FnvHashMap<FolderHash, ImapFolder>>>, pub folders: Arc<RwLock<FnvHashMap<FolderHash, ImapFolder>>>,
pub sender: RefreshEventConsumer, pub sender: RefreshEventConsumer,
pub work_context: WorkContext, pub work_context: WorkContext,
} }
@ -71,7 +71,7 @@ pub fn poll_with_examine(kit: ImapWatchKit) -> Result<()> {
.send((thread_id, "sleeping...".to_string())) .send((thread_id, "sleeping...".to_string()))
.unwrap(); .unwrap();
std::thread::sleep(std::time::Duration::from_millis(5 * 60 * 1000)); std::thread::sleep(std::time::Duration::from_millis(5 * 60 * 1000));
let folders = folders.lock().unwrap(); let folders = folders.read().unwrap();
for folder in folders.values() { for folder in folders.values() {
work_context work_context
.set_status .set_status
@ -109,7 +109,7 @@ pub fn idle(kit: ImapWatchKit) -> Result<()> {
} }
let thread_id: std::thread::ThreadId = std::thread::current().id(); let thread_id: std::thread::ThreadId = std::thread::current().id();
let folder: ImapFolder = folders let folder: ImapFolder = folders
.lock() .read()
.unwrap() .unwrap()
.values() .values()
.find(|f| f.parent.is_none() && f.path().eq_ignore_ascii_case("INBOX")) .find(|f| f.parent.is_none() && f.path().eq_ignore_ascii_case("INBOX"))
@ -207,7 +207,7 @@ pub fn idle(kit: ImapWatchKit) -> Result<()> {
if now.duration_since(watch) >= _5_mins { if now.duration_since(watch) >= _5_mins {
/* Time to poll all inboxes */ /* Time to poll all inboxes */
let mut conn = main_conn.lock().unwrap(); let mut conn = main_conn.lock().unwrap();
for folder in folders.lock().unwrap().values() { for folder in folders.read().unwrap().values() {
work_context work_context
.set_status .set_status
.send(( .send((