Skip to content

Commit 0c72716

Browse files
author
Zelda Hessler
authored
Fix panic when merging a stalled stream log to an empty LogBuffer (#3744)
## Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If it fixes an open issue, please link to the issue here --> this fixes an issue submitted by an internal user. ## Description <!--- Describe your changes in detail --> If the first event pushed into the log underlying stalled stream protection, then the log manager would panic. This is because it was expecting to merge the new event in but there was nothing to merge it with. To fix this, I updated the code to `push` the event when the log manager is empty, instead of merging it. ## Testing <!--- Please describe in detail how you tested your changes --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ran existing tests and wrote a new test exercising the previous-panicking code. ## Checklist <!--- If a checkbox below is not applicable, then please DELETE it rather than leaving it unchecked --> - [x] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates - [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
1 parent 2295d5b commit 0c72716

File tree

4 files changed

+43
-12
lines changed

4 files changed

+43
-12
lines changed

CHANGELOG.next.toml

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,17 @@
1414
[[smithy-rs]]
1515
message = "Support `stringArray` type in endpoints params"
1616
references = ["smithy-rs#3742"]
17-
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client"}
17+
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" }
1818
author = "landonxjames"
19+
20+
[[aws-sdk-rust]]
21+
message = "Fix bug where stalled stream protection would panic with an underflow if the first event was logged too soon."
22+
references = ["smithy-rs#3744"]
23+
meta = { "breaking" = false, "tada" = false, "bug" = true }
24+
author = "Velfi"
25+
26+
[[smithy-rs]]
27+
message = "Fix bug where stalled stream protection would panic with an underflow if the first event was logged too soon."
28+
references = ["smithy-rs#3744"]
29+
meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" }
30+
author = "Velfi"

rust-runtime/Cargo.lock

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

rust-runtime/aws-smithy-runtime/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "aws-smithy-runtime"
3-
version = "1.6.1"
3+
version = "1.6.2"
44
authors = ["AWS Rust SDK Team <aws-sdk-rust@amazon.com>", "Zelda Hessler <zhessler@amazon.com>"]
55
description = "The new smithy runtime crate"
66
edition = "2021"

rust-runtime/aws-smithy-runtime/src/client/http/body/minimum_throughput/throughput.rs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ struct LogBuffer<const N: usize> {
181181
// polling gap. Once the length reaches N, it will never change again.
182182
length: usize,
183183
}
184+
184185
impl<const N: usize> LogBuffer<N> {
185186
fn new() -> Self {
186187
Self {
@@ -247,6 +248,11 @@ impl<const N: usize> LogBuffer<N> {
247248
}
248249
counts
249250
}
251+
252+
/// If this LogBuffer is empty, returns `true`. Else, returns `false`.
253+
fn is_empty(&self) -> bool {
254+
self.length == 0
255+
}
250256
}
251257

252258
/// Report/summary of all the events in a time window.
@@ -334,7 +340,11 @@ impl ThroughputLogs {
334340

335341
fn push(&mut self, now: SystemTime, value: Bin) {
336342
self.catch_up(now);
337-
self.buffer.tail_mut().merge(value);
343+
if self.buffer.is_empty() {
344+
self.buffer.push(value)
345+
} else {
346+
self.buffer.tail_mut().merge(value);
347+
}
338348
self.buffer.fill_gaps();
339349
}
340350

@@ -552,4 +562,13 @@ mod test {
552562
let report = logs.report(start + Duration::from_millis(999));
553563
assert_eq!(ThroughputReport::Pending, report);
554564
}
565+
566+
#[test]
567+
fn test_first_push_succeeds_although_time_window_has_not_elapsed() {
568+
let t0 = SystemTime::UNIX_EPOCH;
569+
let t1 = t0 + Duration::from_secs(1);
570+
let mut tl = ThroughputLogs::new(Duration::from_secs(1), t1);
571+
572+
tl.push_pending(t0);
573+
}
555574
}

0 commit comments

Comments
 (0)