[arrow] branch master updated: ARROW-1380: [Plasma] Fix "still reachable" valgrind warnings when PLASMA_VALGRIND=1

[we...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

wesm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new c6d6164  ARROW-1380: [Plasma] Fix "still reachable" valgrind warnings when PLASMA_VALGRIND=1
c6d6164 is described below

commit c6d6164b8fc443953cc211f241a1076eda883e76
Author: Lukasz A. Bartnik <l....@gmail.com>
AuthorDate: Thu Sep 13 10:18:08 2018 -0400

    ARROW-1380: [Plasma] Fix "still reachable" valgrind warnings when PLASMA_VALGRIND=1
    
    ARROW-1380: Fix "still reachable" valgrind warnings when PLASMA_VALGRIND=1
    
    Author: Lukasz A. Bartnik <l....@gmail.com>
    
    Closes #2466 from lbartnik/fix-1380 and squashes the following commits:
    
    f66a63c4a <Lukasz A. Bartnik> condition valgrind-related sleep on USE_VALGRIND
    6c803d550 <Lukasz A. Bartnik> fix style issues
    64e8ff931 <Lukasz A. Bartnik> remove exit() from signal handler
    c1087f368 <Lukasz A. Bartnik> deallocate the runner
    09ced7f4e <Lukasz A. Bartnik> deallocate stream
    5959300d1 <Lukasz A. Bartnik> add a timeout to mitigate race condition
    5e5243274 <Lukasz A. Bartnik> let the ae event loop shutdown gracefully
---
 cpp/src/plasma/events.cc            |  7 +++----
 cpp/src/plasma/events.h             |  2 ++
 cpp/src/plasma/malloc.cc            |  7 +++++++
 cpp/src/plasma/store.cc             | 12 ++++++++----
 python/pyarrow/tests/test_plasma.py |  5 +++++
 5 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/cpp/src/plasma/events.cc b/cpp/src/plasma/events.cc
index ce29e6c..44637da 100644
--- a/cpp/src/plasma/events.cc
+++ b/cpp/src/plasma/events.cc
@@ -68,10 +68,9 @@ void EventLoop::RemoveFileEvent(int fd) {
 
 void EventLoop::Start() { aeMain(loop_); }
 
-void EventLoop::Stop() {
-  aeStop(loop_);
-  aeDeleteEventLoop(loop_);
-}
+void EventLoop::Stop() { aeStop(loop_); }
+
+void EventLoop::Shutdown() { aeDeleteEventLoop(loop_); }
 
 int64_t EventLoop::AddTimer(int64_t timeout, const TimerCallback& callback) {
   auto data = std::unique_ptr<TimerCallback>(new TimerCallback(callback));
diff --git a/cpp/src/plasma/events.h b/cpp/src/plasma/events.h
index 419c2eb..5b69743 100644
--- a/cpp/src/plasma/events.h
+++ b/cpp/src/plasma/events.h
@@ -88,6 +88,8 @@ class EventLoop {
   /// \brief Stop the event loop
   void Stop();
 
+  void Shutdown();
+
  private:
   static void FileEventCallback(aeEventLoop* loop, int fd, void* context, int events);
 
diff --git a/cpp/src/plasma/malloc.cc b/cpp/src/plasma/malloc.cc
index 3df0892..32e2a82 100644
--- a/cpp/src/plasma/malloc.cc
+++ b/cpp/src/plasma/malloc.cc
@@ -122,6 +122,13 @@ int create_buffer(int64_t size) {
       return -1;
     }
   }
+  int ret = dup(fd);
+  if (ret < 0) {
+    ARROW_LOG(FATAL) << "failed to dup the descriptor";
+  } else {
+    fclose(file);
+    fd = ret;
+  }
 #endif
   return fd;
 }
diff --git a/cpp/src/plasma/store.cc b/cpp/src/plasma/store.cc
index 58f8d01..a2bf3b0 100644
--- a/cpp/src/plasma/store.cc
+++ b/cpp/src/plasma/store.cc
@@ -857,8 +857,10 @@ class PlasmaStoreRunner {
     loop_->Start();
   }
 
+  void Stop() { loop_->Stop(); }
+
   void Shutdown() {
-    loop_->Stop();
+    loop_->Shutdown();
     loop_ = nullptr;
     store_ = nullptr;
   }
@@ -873,10 +875,8 @@ static std::unique_ptr<PlasmaStoreRunner> g_runner = nullptr;
 void HandleSignal(int signal) {
   if (signal == SIGTERM) {
     if (g_runner != nullptr) {
-      g_runner->Shutdown();
+      g_runner->Stop();
     }
-    // Report "success" to valgrind.
-    exit(0);
   }
 }
 
@@ -981,4 +981,8 @@ int main(int argc, char* argv[]) {
   ARROW_LOG(DEBUG) << "starting server listening on " << socket_name;
   plasma::StartServer(socket_name, system_memory, plasma_directory, hugepages_enabled,
                       use_one_memory_mapped_file);
+  plasma::g_runner->Shutdown();
+  plasma::g_runner = nullptr;
+
+  return 0;
 }
diff --git a/python/pyarrow/tests/test_plasma.py b/python/pyarrow/tests/test_plasma.py
index 2510298..061acd5 100644
--- a/python/pyarrow/tests/test_plasma.py
+++ b/python/pyarrow/tests/test_plasma.py
@@ -127,6 +127,11 @@ class TestPlasmaClient(object):
             # Check that the Plasma store is still alive.
             assert self.p.poll() is None
             # Ensure Valgrind and/or coverage have a clean exit
+            # Valgrind misses SIGTERM if it is delivered before the
+            # event loop is ready; this race condition is mitigated
+            # but not solved by time.sleep().
+            if USE_VALGRIND:
+                time.sleep(1.0)
             self.p.send_signal(signal.SIGTERM)
             if sys.version_info >= (3, 3):
                 self.p.wait(timeout=5)