[27/28] impala git commit: IMPALA-6824: Fix crash in RuntimeProfile::EventSequence::AddNewerEvents()

[ta...@apache.org]

IMPALA-6824: Fix crash in RuntimeProfile::EventSequence::AddNewerEvents()

When events_ is empty in runtime-profile-counters.h:347, events_.back()
is undefined. This can lead to a crash. The fix is to check for
events_.empty() before calling back().

This change adds a backend test to make sure that updating empty event
sequences doesn't crash.

Change-Id: I76d30517544e8aa40e7d041f6a65a5dd361ae3c1
Reviewed-on: http://gerrit.cloudera.org:8080/9951
Reviewed-by: Lars Volker <lv...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/d8e53d03
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/d8e53d03
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/d8e53d03

Branch: refs/heads/2.x
Commit: d8e53d03932dd4dd14b7d4f976fb552d2d621ed6
Parents: d73eb3b
Author: Lars Volker <lv...@cloudera.com>
Authored: Sun Apr 8 22:58:39 2018 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Wed Apr 11 22:56:01 2018 +0000

----------------------------------------------------------------------
 be/src/util/runtime-profile-counters.h |  4 ++--
 be/src/util/runtime-profile-test.cc    | 32 +++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/d8e53d03/be/src/util/runtime-profile-counters.h
----------------------------------------------------------------------
diff --git a/be/src/util/runtime-profile-counters.h b/be/src/util/runtime-profile-counters.h
index c8d8bcd..52410b5 100644
--- a/be/src/util/runtime-profile-counters.h
+++ b/be/src/util/runtime-profile-counters.h
@@ -344,10 +344,10 @@ class RuntimeProfile::EventSequence {
     DCHECK_EQ(timestamps.size(), labels.size());
     DCHECK(std::is_sorted(timestamps.begin(), timestamps.end()));
     boost::lock_guard<SpinLock> event_lock(lock_);
-    int64_t last_timestamp = events_.back().second;
+    int64_t last_timestamp = events_.empty() ? 0 : events_.back().second;
     for (int64_t i = 0; i < timestamps.size(); ++i) {
       if (timestamps[i] <= last_timestamp) continue;
-      events_.push_back(make_pair(labels[i], timestamps[i]));
+      events_.emplace_back(labels[i], timestamps[i]);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/impala/blob/d8e53d03/be/src/util/runtime-profile-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/runtime-profile-test.cc b/be/src/util/runtime-profile-test.cc
index 0955203..b194cc9 100644
--- a/be/src/util/runtime-profile-test.cc
+++ b/be/src/util/runtime-profile-test.cc
@@ -613,6 +613,38 @@ TEST(CountersTest, EventSequences) {
   }
 }
 
+TEST(CountersTest, UpdateEmptyEventSequence) {
+  // IMPALA-6824: This test makes sure that adding events to an empty event sequence does
+  // not crash.
+  ObjectPool pool;
+
+  // Create the profile to send in the update and add some events.
+  RuntimeProfile* profile = RuntimeProfile::Create(&pool, "Profile");
+  RuntimeProfile::EventSequence* seq = profile->AddEventSequence("event sequence");
+  seq->Start();
+  // Sleep for 10ms to make sure the events are logged at a time > 0.
+  SleepForMs(10);
+  seq->MarkEvent("aaaa");
+  seq->MarkEvent("bbbb");
+
+  vector<RuntimeProfile::EventSequence::Event> events;
+  seq->GetEvents(&events);
+  EXPECT_EQ(2, events.size());
+
+  TRuntimeProfileTree thrift_profile;
+  profile->ToThrift(&thrift_profile);
+
+  // Create the profile that will be updated and add the empty event sequence to it.
+  RuntimeProfile* updated_profile = RuntimeProfile::Create(&pool, "Updated Profile");
+  seq = updated_profile->AddEventSequence("event sequence");
+  updated_profile->Update(thrift_profile);
+
+  // Verify that the events have been updated successfully.
+  events.clear();
+  seq->GetEvents(&events);
+  EXPECT_EQ(2, events.size());
+}
+
 void ValidateSampler(const StreamingSampler<int, 10>& sampler, int expected_num,
     int expected_period, int expected_delta) {
   const int* samples = NULL;