From ce278aa06ac72802df97a324c149cae73ef2aa6e Mon Sep 17 00:00:00 2001
From: Robert Nishihara <robertnishihara@gmail.com>
Date: Sat, 30 Sep 2017 00:11:09 -0700
Subject: [PATCH] Fix valgrind tests. (#1037)

* Comment out local scheduler valgrind test.

* Fix free/delete error.

* More free -> delete errors

* One more free -> delete and also clean up callback state in plasma manager.

* Add set -x to run_valgrind scripts.

* Fix valgrind error in CreateLocalSchedulerInfoMessage.
---
 .travis.yml                              |  2 +-
 src/common/state/redis.cc                | 12 ++++++++----
 src/common/test/run_valgrind.sh          |  2 ++
 src/local_scheduler/test/run_valgrind.sh |  2 ++
 src/plasma/plasma_manager.cc             |  9 +++++----
 src/plasma/test/run_valgrind.sh          |  2 ++
 6 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 49d049516..7abd009a6 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -54,13 +54,13 @@ matrix:
         - export PATH="$HOME/miniconda/bin:$PATH"
         - ./.travis/install-ray.sh
 
+      script:
         - cd python/ray/core
         - bash ../../../src/common/test/run_valgrind.sh
         - bash ../../../src/plasma/test/run_valgrind.sh
         - bash ../../../src/local_scheduler/test/run_valgrind.sh
         - cd ../../..
 
-      script:
         - python ./python/ray/plasma/test/test.py valgrind
         - python ./python/ray/local_scheduler/test/test.py valgrind
         - python ./python/ray/global_scheduler/test/test.py valgrind
diff --git a/src/common/state/redis.cc b/src/common/state/redis.cc
index d4e0b01c9..e56b343f3 100644
--- a/src/common/state/redis.cc
+++ b/src/common/state/redis.cc
@@ -1378,11 +1378,15 @@ void redis_local_scheduler_table_send_info(TableCallbackData *callback_data) {
 
 void redis_local_scheduler_table_disconnect(DBHandle *db) {
   flatbuffers::FlatBufferBuilder fbb;
-  LocalSchedulerInfoMessageBuilder builder(fbb);
-  builder.add_db_client_id(to_flatbuf(fbb, db->client));
-  builder.add_is_dead(true);
-  auto message = builder.Finish();
+  /* Create the flatbuffers message. */
+  double empty_array[] = {};
+  /* Most of the flatbuffer message fields don't matter here. Only the
+   * db_client_id and the is_dead field matter. */
+  auto message = CreateLocalSchedulerInfoMessage(
+      fbb, to_flatbuf(fbb, db->client), 0, 0, 0,
+      fbb.CreateVector(empty_array, 0), fbb.CreateVector(empty_array, 0), true);
   fbb.Finish(message);
+
   redisReply *reply = (redisReply *) redisCommand(
       db->sync_context, "PUBLISH local_schedulers %b", fbb.GetBufferPointer(),
       fbb.GetSize());
diff --git a/src/common/test/run_valgrind.sh b/src/common/test/run_valgrind.sh
index 7e700f574..9424d9a8e 100644
--- a/src/common/test/run_valgrind.sh
+++ b/src/common/test/run_valgrind.sh
@@ -2,6 +2,8 @@
 
 # This needs to be run in the build tree, which is normally ray/build
 
+set -x
+
 # Cause the script to exit if a single command fails.
 set -e
 
diff --git a/src/local_scheduler/test/run_valgrind.sh b/src/local_scheduler/test/run_valgrind.sh
index 9c4e49a3f..180f039c7 100644
--- a/src/local_scheduler/test/run_valgrind.sh
+++ b/src/local_scheduler/test/run_valgrind.sh
@@ -2,6 +2,8 @@
 
 # This needs to be run in the build tree, which is normally ray/build
 
+set -x
+
 # Cause the script to exit if a single command fails.
 set -e
 
diff --git a/src/plasma/plasma_manager.cc b/src/plasma/plasma_manager.cc
index 9db4265b3..0612d53a0 100644
--- a/src/plasma/plasma_manager.cc
+++ b/src/plasma/plasma_manager.cc
@@ -561,6 +561,7 @@ void PlasmaManagerState_free(PlasmaManagerState *state) {
 
   ARROW_CHECK_OK(state->plasma_conn->Disconnect());
   delete state->plasma_conn;
+  destroy_outstanding_callbacks(state->loop);
   event_loop_destroy(state->loop);
   delete state;
 }
@@ -671,7 +672,7 @@ void send_queued_request(event_loop *loop,
       conn->pending_object_transfers.erase(buf->object_id);
     }
     conn->transfer_queue.pop_front();
-    free(buf);
+    delete buf;
   }
 }
 
@@ -727,7 +728,7 @@ void process_data_chunk(event_loop *loop,
       conn->manager_state->plasma_conn->Release(buf->object_id.to_plasma_id()));
   /* Remove the request buffer used for reading this object's data. */
   conn->transfer_queue.pop_front();
-  free(buf);
+  delete buf;
   /* Switch to listening for requests from this socket, instead of reading
    * object data. */
   event_loop_remove_file(loop, data_sock);
@@ -749,7 +750,7 @@ void ignore_data_chunk(event_loop *loop,
   }
 
   free(buf->data);
-  free(buf);
+  delete buf;
   /* Switch to listening for requests from this socket, instead of reading
    * object data. */
   event_loop_remove_file(loop, data_sock);
@@ -1444,7 +1445,7 @@ void ClientConnection_free(ClientConnection *client_conn) {
   /* Close the manager connection and free the remaining state. */
   close(client_conn->fd);
   free(client_conn->ip_addr_port);
-  free(client_conn);
+  delete client_conn;
 }
 
 void handle_new_client(event_loop *loop,
diff --git a/src/plasma/test/run_valgrind.sh b/src/plasma/test/run_valgrind.sh
index 8b7e3b0fb..58263a932 100644
--- a/src/plasma/test/run_valgrind.sh
+++ b/src/plasma/test/run_valgrind.sh
@@ -1,5 +1,7 @@
 #!/usr/bin/env bash
 
+set -x
+
 # Cause the script to exit if a single command fails.
 set -e