Skip to content

Commit

Permalink
Fix panic from repeat clones of TypeErasedBox (#2842)
Browse files Browse the repository at this point in the history
This PR fixes a panic that would result from cloning a cloneable
`TypeErasedBox` twice.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
  • Loading branch information
jdisanti authored Jul 13, 2023
1 parent c1a1dae commit 3d1c34d
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 4 deletions.
7 changes: 7 additions & 0 deletions rust-runtime/aws-smithy-types/src/config_bag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,13 @@ mod test {
layer_1.unset::<TestStr>();
assert!(layer_1.try_clone().unwrap().load::<TestStr>().is_none());

// It is cloneable multiple times in succession
let _ = layer_1
.try_clone()
.expect("clone 1")
.try_clone()
.expect("clone 2");

#[derive(Clone, Debug)]
struct Rope(String);
impl Storable for Rope {
Expand Down
17 changes: 13 additions & 4 deletions rust-runtime/aws-smithy-types/src/type_erasure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,13 @@ impl TypeErasedBox {

impl fmt::Debug for TypeErasedBox {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str("TypeErasedBox:")?;
f.write_str("TypeErasedBox[")?;
if self.clone.is_some() {
f.write_str("Clone")?;
} else {
f.write_str("!Clone")?;
}
f.write_str("]:")?;
(self.debug)(&self.field, f)
}
}
Expand All @@ -145,7 +151,7 @@ impl TypeErasedBox {
fmt::Debug::fmt(value.downcast_ref::<T>().expect("type-checked"), f)
};
let clone = |value: &Box<dyn Any + Send + Sync>| {
TypeErasedBox::new(value.downcast_ref::<T>().expect("typechecked").clone())
TypeErasedBox::new_with_clone(value.downcast_ref::<T>().expect("typechecked").clone())
};
Self {
field: Box::new(value),
Expand Down Expand Up @@ -292,8 +298,11 @@ mod tests {
.0 = "3";

let bar_erased = bar.erase();
assert_eq!("TypeErasedBox:Foo(\"3\")", format!("{foo_erased:?}"));
assert_eq!("TypeErasedBox:Bar(2)", format!("{bar_erased:?}"));
assert_eq!(
"TypeErasedBox[!Clone]:Foo(\"3\")",
format!("{foo_erased:?}")
);
assert_eq!("TypeErasedBox[!Clone]:Bar(2)", format!("{bar_erased:?}"));

let bar_erased = TypedBox::<Foo>::assume_from(bar_erased).expect_err("it's not a Foo");
let mut bar = TypedBox::<Bar>::assume_from(bar_erased).expect("it's a Bar");
Expand Down

0 comments on commit 3d1c34d

Please sign in to comment.