From fe82e1704686ccbcdabaa1715cf30c5ce15cc17a Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Mon, 20 Dec 2021 11:39:12 -0800 Subject: [PATCH] fix(subscriber): use saturating arithmetic for attribute updates (#234) Currently, an issue exists where a subtraction attribute update for a resource or async op attribute may overflow, if a previous add update was dropped due to event buffer capacity. This may result in the aggregator task panicking. See https://github.com/tokio-rs/console/issues/180#issuecomment-998132325 for details. This branch changes all resource updates to use saturating arithmetic. In practice, I think it's _much_ more likely for a subtract to overflow due to missed adds than it is for an add to overflow due to missed subs, but I made the addition saturating as well, just in case. We should probably also increase the default event buffer capacity, since it seems like lots of people are hitting problems with this... --- console-subscriber/src/aggregator/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/console-subscriber/src/aggregator/mod.rs b/console-subscriber/src/aggregator/mod.rs index ecf3b630d..4397dc990 100644 --- a/console-subscriber/src/aggregator/mod.rs +++ b/console-subscriber/src/aggregator/mod.rs @@ -1074,9 +1074,9 @@ fn update_attribute(attribute: &mut Attribute, update: &AttributeUpdate) { (Some(DebugVal(v)), Some(DebugVal(upd))) => *v = upd, (Some(U64Val(v)), Some(U64Val(upd))) => match update.op { - Some(AttributeUpdateOp::Add) => *v += upd, + Some(AttributeUpdateOp::Add) => *v = v.saturating_add(upd), - Some(AttributeUpdateOp::Sub) => *v -= upd, + Some(AttributeUpdateOp::Sub) => *v = v.saturating_sub(upd), Some(AttributeUpdateOp::Override) => *v = upd, @@ -1087,9 +1087,9 @@ fn update_attribute(attribute: &mut Attribute, update: &AttributeUpdate) { }, (Some(I64Val(v)), Some(I64Val(upd))) => match update.op { - Some(AttributeUpdateOp::Add) => *v += upd, + Some(AttributeUpdateOp::Add) => *v = v.saturating_add(upd), - Some(AttributeUpdateOp::Sub) => *v -= upd, + Some(AttributeUpdateOp::Sub) => *v = v.saturating_sub(upd), Some(AttributeUpdateOp::Override) => *v = upd,