Fix race condition in alloc_ongoing()

Hold the same write lock while checking if ongoing
process is already allocated and while allocating it.
Otherwise it is possible for two parallel processes
running alloc_ongoing() to decide that no ongoing
process is allocated and allocate two ongoing processes.
This commit is contained in:
link2xt
2022-05-15 23:24:24 +00:00
committed by holger krekel
parent 380d7e66b5
commit 6d189dab98
3 changed files with 9 additions and 18 deletions

View File

@@ -24,8 +24,12 @@
- node: throw error when getting context with an invalid account id - node: throw error when getting context with an invalid account id
- node: throw error when instanciating a wrapper class on `null` (Context, Message, Chat, ChatList and so on) - node: throw error when instanciating a wrapper class on `null` (Context, Message, Chat, ChatList and so on)
- use same contact-color if email address differ only in upper-/lowercase #3327 - use same contact-color if email address differ only in upper-/lowercase #3327
- fix race condition in ongoing process (import/export, configuration) allocation
- repair encrypted mails "mixed up" by Google Workspace "Append footer" function #3315 - repair encrypted mails "mixed up" by Google Workspace "Append footer" function #3315
### Removed
- node: remove unmaintained coverage scripts
## 1.80.0 ## 1.80.0

View File

@@ -278,13 +278,11 @@ impl Context {
// Ongoing process allocation/free/check // Ongoing process allocation/free/check
pub(crate) async fn alloc_ongoing(&self) -> Result<Receiver<()>> { pub(crate) async fn alloc_ongoing(&self) -> Result<Receiver<()>> {
if self.has_ongoing().await { let mut s = self.running_state.write().await;
if s.ongoing_running || !s.shall_stop_ongoing {
bail!("There is already another ongoing process running."); bail!("There is already another ongoing process running.");
} }
let s_a = &self.running_state;
let mut s = s_a.write().await;
s.ongoing_running = true; s.ongoing_running = true;
s.shall_stop_ongoing = false; s.shall_stop_ongoing = false;
let (sender, receiver) = channel::bounded(1); let (sender, receiver) = channel::bounded(1);
@@ -294,25 +292,16 @@ impl Context {
} }
pub(crate) async fn free_ongoing(&self) { pub(crate) async fn free_ongoing(&self) {
let s_a = &self.running_state; let mut s = self.running_state.write().await;
let mut s = s_a.write().await;
s.ongoing_running = false; s.ongoing_running = false;
s.shall_stop_ongoing = true; s.shall_stop_ongoing = true;
s.cancel_sender.take(); s.cancel_sender.take();
} }
pub(crate) async fn has_ongoing(&self) -> bool {
let s_a = &self.running_state;
let s = s_a.read().await;
s.ongoing_running || !s.shall_stop_ongoing
}
/// Signal an ongoing process to stop. /// Signal an ongoing process to stop.
pub async fn stop_ongoing(&self) { pub async fn stop_ongoing(&self) {
let s_a = &self.running_state; let mut s = self.running_state.write().await;
let mut s = s_a.write().await;
if let Some(cancel) = s.cancel_sender.take() { if let Some(cancel) = s.cancel_sender.take() {
if let Err(err) = cancel.send(()).await { if let Err(err) = cancel.send(()).await {
warn!(self, "could not cancel ongoing: {:?}", err); warn!(self, "could not cancel ongoing: {:?}", err);
@@ -324,7 +313,7 @@ impl Context {
s.shall_stop_ongoing = true; s.shall_stop_ongoing = true;
} else { } else {
info!(self, "No ongoing process to stop.",); info!(self, "No ongoing process to stop.",);
}; }
} }
pub(crate) async fn shall_stop_ongoing(&self) -> bool { pub(crate) async fn shall_stop_ongoing(&self) -> bool {

View File

@@ -741,7 +741,6 @@ mod tests {
); );
let sent = bob.pop_sent_msg().await; let sent = bob.pop_sent_msg().await;
assert!(!bob.ctx.has_ongoing().await);
assert_eq!(sent.recipient(), "alice@example.org".parse().unwrap()); assert_eq!(sent.recipient(), "alice@example.org".parse().unwrap());
let msg = alice.parse_msg(&sent).await; let msg = alice.parse_msg(&sent).await;
assert!(!msg.was_encrypted()); assert!(!msg.was_encrypted());
@@ -1291,7 +1290,6 @@ mod tests {
let bob_chat = Chat::load_from_db(&bob.ctx, bob_chatid).await?; let bob_chat = Chat::load_from_db(&bob.ctx, bob_chatid).await?;
assert!(bob_chat.is_protected()); assert!(bob_chat.is_protected());
assert!(bob_chat.typ == Chattype::Group); assert!(bob_chat.typ == Chattype::Group);
assert!(!bob.ctx.has_ongoing().await);
// On this "happy path", Alice and Bob get only a group-chat where all information are added to. // On this "happy path", Alice and Bob get only a group-chat where all information are added to.
// The one-to-one chats are used internally for the hidden handshake messages, // The one-to-one chats are used internally for the hidden handshake messages,