Skip to content

Commit 943c8a1

Browse files
authored
feat(imex): Cancel BackupProvider when dropped (#4242)
This ensures that the BackupProvider will be stopped as soon as the struct is dropped and the imex progress error event is emitted. This makes it easier to use and also makes sure that the ffi call dc_backup_provider_unref() does not lead to dangling resources.
1 parent 5be558e commit 943c8a1

File tree

5 files changed

+39
-5
lines changed

5 files changed

+39
-5
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
### Changes
66
- Update iroh, remove `default-net` from `[patch.crates-io]` section.
77
- transfer backup: Connect to mutliple provider addresses concurrently. This should speed up connection time significantly on the getter side. #4240
8+
- Make sure BackupProvider is cancelled on drop (or dc_backup_provider_unref). The BackupProvider will now alaway finish with an IMEX event of 1000 or 0, previoulsy it would sometimes finishe with 1000 (success) when it really was 0 (failure). #4242
89

910
## [1.112.1] - 2023-03-27
1011

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,11 @@ strum_macros = "0.24"
8484
tagger = "4.3.4"
8585
textwrap = "0.16.0"
8686
thiserror = "1"
87+
tokio = { version = "1", features = ["fs", "rt-multi-thread", "macros"] }
8788
tokio-io-timeout = "1.2.0"
8889
tokio-stream = { version = "0.1.11", features = ["fs"] }
8990
tokio-tar = { version = "0.3" } # TODO: integrate tokio into async-tar
90-
tokio = { version = "1", features = ["fs", "rt-multi-thread", "macros"] }
91+
tokio-util = "0.7.7"
9192
toml = "0.7"
9293
trust-dns-resolver = "0.22"
9394
url = "2"

deltachat-ffi/deltachat.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2775,6 +2775,12 @@ void dc_backup_provider_wait (dc_backup_provider_t* backup_provider);
27752775
/**
27762776
* Frees a dc_backup_provider_t object.
27772777
*
2778+
* If the provider has not yet finished, as indicated by
2779+
* dc_backup_provider_wait() or the #DC_EVENT_IMEX_PROGRESS event with value
2780+
* of 0 (failed) or 1000 (succeeded), this will also abort any in-progress
2781+
* transfer. If this aborts the provider a #DC_EVENT_IMEX_PROGRESS event with
2782+
* value 0 (failed) will be emitted.
2783+
*
27782784
* @memberof dc_backup_provider_t
27792785
* @param backup_provider The backup provider object as created by
27802786
* dc_backup_provider_new().

src/imex/transfer.rs

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ use tokio::sync::broadcast::error::RecvError;
4444
use tokio::sync::{broadcast, Mutex};
4545
use tokio::task::{JoinHandle, JoinSet};
4646
use tokio_stream::wrappers::ReadDirStream;
47+
use tokio_util::sync::CancellationToken;
4748

4849
use crate::blob::BlobDirContents;
4950
use crate::chat::delete_and_reset_all_device_msgs;
@@ -74,6 +75,8 @@ pub struct BackupProvider {
7475
handle: JoinHandle<Result<()>>,
7576
/// The ticket to retrieve the backup collection.
7677
ticket: Ticket,
78+
/// Guard to cancel the provider on drop.
79+
_drop_guard: tokio_util::sync::DropGuard,
7780
}
7881

7982
impl BackupProvider {
@@ -125,10 +128,12 @@ impl BackupProvider {
125128
return Err(err);
126129
}
127130
};
131+
let drop_token = CancellationToken::new();
128132
let handle = {
129133
let context = context.clone();
134+
let drop_token = drop_token.clone();
130135
tokio::spawn(async move {
131-
let res = Self::watch_provider(&context, provider, cancel_token).await;
136+
let res = Self::watch_provider(&context, provider, cancel_token, drop_token).await;
132137
context.free_ongoing().await;
133138

134139
// Explicit drop to move the guards into this future
@@ -137,7 +142,11 @@ impl BackupProvider {
137142
res
138143
})
139144
};
140-
Ok(Self { handle, ticket })
145+
Ok(Self {
146+
handle,
147+
ticket,
148+
_drop_guard: drop_token.drop_guard(),
149+
})
141150
}
142151

143152
/// Creates the provider task.
@@ -191,8 +200,8 @@ impl BackupProvider {
191200
context: &Context,
192201
mut provider: Provider,
193202
cancel_token: Receiver<()>,
203+
drop_token: CancellationToken,
194204
) -> Result<()> {
195-
// _dbfile exists so we can clean up the file once it is no longer needed
196205
let mut events = provider.subscribe();
197206
let mut total_size = 0;
198207
let mut current_size = 0;
@@ -252,8 +261,12 @@ impl BackupProvider {
252261
},
253262
_ = cancel_token.recv() => {
254263
provider.shutdown();
255-
break Err(anyhow!("BackupSender cancelled"));
264+
break Err(anyhow!("BackupProvider cancelled"));
256265
},
266+
_ = drop_token.cancelled() => {
267+
provider.shutdown();
268+
break Err(anyhow!("BackupProvider dropped"));
269+
}
257270
}
258271
};
259272
match &res {
@@ -660,4 +673,16 @@ mod tests {
660673
assert_eq!(out, EventType::ImexProgress(progress));
661674
}
662675
}
676+
677+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
678+
async fn test_drop_provider() {
679+
let mut tcm = TestContextManager::new();
680+
let ctx = tcm.alice().await;
681+
682+
let provider = BackupProvider::prepare(&ctx).await.unwrap();
683+
drop(provider);
684+
ctx.evtracker
685+
.get_matching(|ev| matches!(ev, EventType::ImexProgress(0)))
686+
.await;
687+
}
663688
}

0 commit comments

Comments
 (0)