From 3495b2fa9d84e8105a79e24e4e9a2f9181318f1a Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 18 Jan 2018 11:14:39 -0600 Subject: [PATCH] Fix problems with SumStats non-cluster.bro script * Add proper namespace scoping to a 'SumStats::process_epoch_result' scheduled event. * Fix iterator invalidation within 'SumStats::process_epoch_result' * Give 'SumStats::process_epoch_result' a copy of the result table so that the SumStats framework can clear the original and move on to the next epoch immediately. * The previous baseline of the basic sumstats unit test did look wrong to me and probably was actually indicative of the iterator invalidation problem. Thanks to Jim Mellander for reporting the issues. --- .../base/frameworks/sumstats/non-cluster.bro | 35 +++++++++---------- .../standalone..stdout | 1 + 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/scripts/base/frameworks/sumstats/non-cluster.bro b/scripts/base/frameworks/sumstats/non-cluster.bro index 9fdd012404..57785a03b2 100644 --- a/scripts/base/frameworks/sumstats/non-cluster.bro +++ b/scripts/base/frameworks/sumstats/non-cluster.bro @@ -6,28 +6,25 @@ event SumStats::process_epoch_result(ss: SumStat, now: time, data: ResultTable) { # TODO: is this the right processing group size? local i = 50; + local keys_to_delete: vector of SumStats::Key = vector(); + for ( key in data ) { ss$epoch_result(now, key, data[key]); - delete data[key]; + keys_to_delete[|keys_to_delete|] = key; - if ( |data| == 0 ) - { - if ( ss?$epoch_finished ) - ss$epoch_finished(now); - - # Now that no data is left we can finish. - return; - } - - i = i-1; - if ( i == 0 ) - { - # TODO: is this the right interval? - schedule 0.01 secs { process_epoch_result(ss, now, data) }; + if ( --i == 0 ) break; - } } + + for ( idx in keys_to_delete ) + delete data[keys_to_delete[idx]]; + + if ( |data| > 0 ) + # TODO: is this the right interval? + schedule 0.01 secs { SumStats::process_epoch_result(ss, now, data) }; + else if ( ss?$epoch_finished ) + ss$epoch_finished(now); } event SumStats::finish_epoch(ss: SumStat) @@ -46,9 +43,9 @@ event SumStats::finish_epoch(ss: SumStat) if ( ss?$epoch_finished ) ss$epoch_finished(now); } - else + else if ( |data| > 0 ) { - event SumStats::process_epoch_result(ss, now, data); + event SumStats::process_epoch_result(ss, now, copy(data)); } } @@ -89,4 +86,4 @@ function request_key(ss_name: string, key: Key): Result else return table(); } - } \ No newline at end of file + } diff --git a/testing/btest/Baseline/scripts.base.frameworks.sumstats.basic/standalone..stdout b/testing/btest/Baseline/scripts.base.frameworks.sumstats.basic/standalone..stdout index 6820df8d93..b345cbf850 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.sumstats.basic/standalone..stdout +++ b/testing/btest/Baseline/scripts.base.frameworks.sumstats.basic/standalone..stdout @@ -1,2 +1,3 @@ Host: 1.2.3.4 - num:5 - sum:221.0 - var:1144.2 - avg:44.2 - max:94.0 - min:5.0 - std_dev:33.8 - unique:4 - hllunique:4 Host: 6.5.4.3 - num:1 - sum:2.0 - var:0.0 - avg:2.0 - max:2.0 - min:2.0 - std_dev:0.0 - unique:1 - hllunique:1 +Host: 7.2.1.5 - num:1 - sum:1.0 - var:0.0 - avg:1.0 - max:1.0 - min:1.0 - std_dev:0.0 - unique:1 - hllunique:1