Skip to content

Commit 529e136

Browse files
committed
ref: Box::pin all the moka init Futures
We can extract this from #1010, and this should help with moka-rs/moka#212 even if we do not (yet) switch to more widespread moka usage.
1 parent 5f67bb7 commit 529e136

File tree

5 files changed

+81
-93
lines changed

5 files changed

+81
-93
lines changed

crates/symbolicator-service/src/services/download/gcs.rs

+16-18
Original file line numberDiff line numberDiff line change
@@ -42,25 +42,23 @@ impl GcsDownloader {
4242
/// If the cache contains a valid token, then this token is returned. Otherwise, a new token is
4343
/// requested from GCS and stored in the cache.
4444
async fn get_token(&self, source_key: &Arc<GcsSourceKey>) -> CacheEntry<Arc<GcsToken>> {
45+
let init = Box::pin(async {
46+
let token = gcs::request_new_token(&self.client, source_key).await;
47+
metric!(counter("source.gcs.token.requests") += 1);
48+
token.map(Arc::new).map_err(CacheError::from)
49+
});
50+
let replace_if = |entry: &CacheEntry<Arc<GcsToken>>| match entry {
51+
Ok(token) => {
52+
let is_expired = token.is_expired();
53+
if !is_expired {
54+
metric!(counter("source.gcs.token.cached") += 1);
55+
}
56+
is_expired
57+
}
58+
Err(_) => true,
59+
};
4560
self.token_cache
46-
.get_with_if(
47-
source_key.clone(),
48-
async {
49-
let token = gcs::request_new_token(&self.client, source_key).await;
50-
metric!(counter("source.gcs.token.requests") += 1);
51-
token.map(Arc::new).map_err(CacheError::from)
52-
},
53-
|entry| match entry {
54-
Ok(token) => {
55-
let is_expired = token.is_expired();
56-
if !is_expired {
57-
metric!(counter("source.gcs.token.cached") += 1);
58-
}
59-
is_expired
60-
}
61-
Err(_) => true,
62-
},
63-
)
61+
.get_with_if(source_key.clone(), init, replace_if)
6462
.await
6563
}
6664

crates/symbolicator-service/src/services/download/s3.rs

+24-25
Original file line numberDiff line numberDiff line change
@@ -58,32 +58,31 @@ impl S3Downloader {
5858
metric!(counter("source.s3.client.cached") += 1);
5959
}
6060

61-
self.client_cache
62-
.get_with_by_ref(key, async {
63-
metric!(counter("source.s3.client.create") += 1);
64-
65-
tracing::debug!(
66-
"Using AWS credentials provider: {:?}",
67-
key.aws_credentials_provider
68-
);
69-
Arc::new(match key.aws_credentials_provider {
70-
AwsCredentialsProvider::Container => {
71-
let provider = LazyCachingCredentialsProvider::builder()
72-
.load(aws_config::ecs::EcsCredentialsProvider::builder().build())
73-
.build();
74-
self.create_s3_client(provider, &key.region).await
75-
}
76-
AwsCredentialsProvider::Static => {
77-
let provider = Credentials::from_keys(
78-
key.access_key.clone(),
79-
key.secret_key.clone(),
80-
None,
81-
);
82-
self.create_s3_client(provider, &key.region).await
83-
}
84-
})
61+
let init = Box::pin(async {
62+
metric!(counter("source.s3.client.create") += 1);
63+
64+
tracing::debug!(
65+
"Using AWS credentials provider: {:?}",
66+
key.aws_credentials_provider
67+
);
68+
Arc::new(match key.aws_credentials_provider {
69+
AwsCredentialsProvider::Container => {
70+
let provider = LazyCachingCredentialsProvider::builder()
71+
.load(aws_config::ecs::EcsCredentialsProvider::builder().build())
72+
.build();
73+
self.create_s3_client(provider, &key.region).await
74+
}
75+
AwsCredentialsProvider::Static => {
76+
let provider = Credentials::from_keys(
77+
key.access_key.clone(),
78+
key.secret_key.clone(),
79+
None,
80+
);
81+
self.create_s3_client(provider, &key.region).await
82+
}
8583
})
86-
.await
84+
});
85+
self.client_cache.get_with_by_ref(key, init).await
8786
}
8887

8988
async fn create_s3_client(

crates/symbolicator-service/src/services/download/sentry.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ impl SentryDownloader {
9898
/// If there are cached search results this skips the actual search.
9999
async fn cached_sentry_search(&self, query: SearchQuery) -> CacheEntry<Vec<SearchResult>> {
100100
let query_ = query.clone();
101-
let init_future = async {
101+
let init = Box::pin(async {
102102
tracing::debug!(
103103
"Fetching list of Sentry debug files from {}",
104104
&query_.index_url
@@ -112,9 +112,9 @@ impl SentryDownloader {
112112
CancelOnDrop::new(self.runtime.spawn(future.bind_hub(sentry::Hub::current())));
113113

114114
future.await.map_err(|_| CacheError::InternalError)?
115-
};
115+
});
116116
self.index_cache
117-
.get_with_if(query, init_future, |entry| entry.is_err())
117+
.get_with_if(query, init, |entry| entry.is_err())
118118
.await
119119
}
120120

crates/symbolicator-service/src/services/symbolication/process_minidump.rs

+29-30
Original file line numberDiff line numberDiff line change
@@ -173,37 +173,36 @@ impl SymbolicatorSymbolProvider {
173173
/// Fetches CFI for the given module, parses it into a `SymbolFile`, and stores it internally.
174174
async fn load_cfi_module(&self, module: &(dyn Module + Sync)) -> FetchedCfiCache {
175175
let key = LookupKey::new(module);
176-
self.cficaches
177-
.get_with_by_ref(&key, async {
178-
let sources = self.sources.clone();
179-
let scope = self.scope.clone();
180-
181-
let identifier = ObjectId {
182-
code_id: key.code_id.clone(),
183-
code_file: Some(module.code_file().into_owned()),
184-
debug_id: key.debug_id,
185-
debug_file: module
186-
.debug_file()
187-
.map(|debug_file| debug_file.into_owned()),
188-
debug_checksum: None,
176+
let load = Box::pin(async {
177+
let sources = self.sources.clone();
178+
let scope = self.scope.clone();
179+
180+
let identifier = ObjectId {
181+
code_id: key.code_id.clone(),
182+
code_file: Some(module.code_file().into_owned()),
183+
debug_id: key.debug_id,
184+
debug_file: module
185+
.debug_file()
186+
.map(|debug_file| debug_file.into_owned()),
187+
debug_checksum: None,
188+
object_type: self.object_type,
189+
};
190+
191+
self.cficache_actor
192+
.fetch(FetchCfiCache {
189193
object_type: self.object_type,
190-
};
191-
192-
self.cficache_actor
193-
.fetch(FetchCfiCache {
194-
object_type: self.object_type,
195-
identifier,
196-
sources,
197-
scope,
198-
})
199-
// NOTE: this `bind_hub` is important!
200-
// `load_cfi_module` is being called concurrently from `rust-minidump` via
201-
// `join_all`. We do need proper isolation of any async task that might
202-
// manipulate any Sentry scope.
203-
.bind_hub(Hub::new_from_top(Hub::current()))
204-
.await
205-
})
206-
.await
194+
identifier,
195+
sources,
196+
scope,
197+
})
198+
// NOTE: this `bind_hub` is important!
199+
// `load_cfi_module` is being called concurrently from `rust-minidump` via
200+
// `join_all`. We do need proper isolation of any async task that might
201+
// manipulate any Sentry scope.
202+
.bind_hub(Hub::new_from_top(Hub::current()))
203+
.await
204+
});
205+
self.cficaches.get_with_by_ref(&key, load).await
207206
}
208207
}
209208

crates/symbolicator-service/src/services/symcaches/mod.rs

+9-17
Original file line numberDiff line numberDiff line change
@@ -443,20 +443,19 @@ mod tests {
443443

444444
// Create the symcache for the first time. Since the bcsymbolmap is not available, names in the
445445
// symcache will be obfuscated.
446-
let owned_symcache = symcache_actor
446+
let symcache = symcache_actor
447447
.fetch(fetch_symcache.clone())
448448
.await
449449
.cache
450-
.ok()
451450
.unwrap();
452451

453-
let symcache = owned_symcache.get();
454-
let sl = symcache.lookup(0x5a75).next().unwrap();
452+
let sl = symcache.get().lookup(0x5a75).next().unwrap();
455453
assert_eq!(
456454
sl.file().unwrap().full_path(),
457455
"__hidden#41_/__hidden#41_/__hidden#42_"
458456
);
459457
assert_eq!(sl.function().name(), "__hidden#0_");
458+
drop(symcache);
460459

461460
// Copy the plist and bcsymbolmap to the temporary symbol directory so that the SymCacheActor can find them.
462461
fs::copy(
@@ -472,37 +471,30 @@ mod tests {
472471
.unwrap();
473472

474473
// Create the symcache for the second time. Even though the bcsymbolmap is now available, its absence should
475-
// still be cached and the SymcacheActor should make no attempt to download it. Therefore, the names should
474+
// still be cached and the SymCacheActor should make no attempt to download it. Therefore, the names should
476475
// be obfuscated like before.
477-
let owned_symcache = symcache_actor
476+
let symcache = symcache_actor
478477
.fetch(fetch_symcache.clone())
479478
.await
480479
.cache
481-
.ok()
482480
.unwrap();
483481

484-
let symcache = owned_symcache.get();
485-
let sl = symcache.lookup(0x5a75).next().unwrap();
482+
let sl = symcache.get().lookup(0x5a75).next().unwrap();
486483
assert_eq!(
487484
sl.file().unwrap().full_path(),
488485
"__hidden#41_/__hidden#41_/__hidden#42_"
489486
);
490487
assert_eq!(sl.function().name(), "__hidden#0_");
488+
drop(symcache);
491489

492490
// Sleep long enough for the negative cache entry to become invalid.
493491
std::thread::sleep(TIMEOUT);
494492

495493
// Create the symcache for the third time. This time, the bcsymbolmap is downloaded and the names in the
496494
// symcache are unobfuscated.
497-
let owned_symcache = symcache_actor
498-
.fetch(fetch_symcache.clone())
499-
.await
500-
.cache
501-
.ok()
502-
.unwrap();
495+
let symcache = symcache_actor.fetch(fetch_symcache).await.cache.unwrap();
503496

504-
let symcache = owned_symcache.get();
505-
let sl = symcache.lookup(0x5a75).next().unwrap();
497+
let sl = symcache.get().lookup(0x5a75).next().unwrap();
506498
assert_eq!(
507499
sl.file().unwrap().full_path(),
508500
"/Users/philipphofmann/git-repos/sentry-cocoa/Sources/Sentry/SentryMessage.m"

0 commit comments

Comments
 (0)