ARROW-5930: [Python] Make Flight server init phase explicit

Should fix the sporadic crashes in test_flight.py.

.travis.yml

@@ -120,7 +120,7 @@ matrix:
     # Only run Plasma tests with valgrind in one of the Python builds because
     # they are slow
     - export PLASMA_VALGRIND=1
-    - $TRAVIS_BUILD_DIR/ci/travis_script_python.sh 3.6 || travis_terminate 1
+    - $TRAVIS_BUILD_DIR/ci/travis_script_python.sh 3.6
     - $TRAVIS_BUILD_DIR/ci/travis_upload_cpp_coverage.sh
   - name: "[OS X] C++ w/ XCode 9.3"
     compiler: clang

ci/travis/after-failure.sh

@@ -26,9 +26,10 @@ if [[ "${TRAVIS_OS_NAME}" = "osx" ]]; then
   ls -la ~/Library/Logs/DiagnosticReports/
   cat ~/Library/Logs/DiagnosticReports/*.crash
 else
-  COREFILE=$(find . -maxdepth 2 -name "core*" | head -n 1)
+  ls -fd1 /tmp/core.*
+  COREFILE=$(ls -fd1 /tmp/core.* | head -n 1)
   if [[ -f "$COREFILE" ]]; then
-    gdb -c "$COREFILE" example -ex "thread apply all bt" -ex "set pagination 0" -batch
+    gdb -c "$COREFILE" $TRAVIS_BUILD_DIR/current-exe -ex "thread apply all bt" -ex "set pagination 0" -batch
   fi
 fi
 

ci/travis/before-install.sh

@@ -20,10 +20,13 @@ set -ex
 
 eval "${MATRIX_EVAL}"
 
-# Enables core files
+# Enable core files
 ulimit -c unlimited -S
 
 if [[ "${TRAVIS_OS_NAME}" == "linux" ]]; then
+  # Remove apport's core_pattern
+  sudo bash -c "echo '/tmp/core.%p.%E' > /proc/sys/kernel/core_pattern"
+
   echo -e 'Acquire::Retries 10; Acquire::http::Timeout \"20\";' | \
     sudo tee /etc/apt/apt.conf.d/99-travis-retry
   sudo apt-get update -qq

ci/travis_script_python.sh

@@ -192,11 +192,14 @@ if [ $TRAVIS_OS_NAME == "linux" ]; then
     sudo bash -c "echo 2048 > /proc/sys/vm/nr_hugepages"
 fi
 
+# For core dump analysis
+ln -sf `which python` $TRAVIS_BUILD_DIR/current-exe
+
 # Need to run tests from the source tree for Cython coverage and conftest.py
 if [ "$ARROW_TRAVIS_COVERAGE" == "1" ]; then
     # Output Python coverage data in a persistent place
     export COVERAGE_FILE=$ARROW_PYTHON_COVERAGE_FILE
-    coverage run --append -m pytest $PYARROW_PYTEST_FLAGS pyarrow/tests
+    python -m coverage run --append -m pytest $PYARROW_PYTEST_FLAGS pyarrow/tests
 else
     python -m pytest $PYARROW_PYTEST_FLAGS pyarrow/tests
 fi

cpp/src/arrow/flight/perf-server.cc

@@ -196,7 +196,7 @@ std::unique_ptr<arrow::flight::FlightPerfServer> g_server;
 
 void Shutdown(int signal) {
   if (g_server != nullptr) {
-    g_server->Shutdown();
+    ARROW_CHECK_OK(g_server->Shutdown());
   }
 }
 

cpp/src/arrow/flight/server.cc

@@ -584,9 +584,13 @@ Status FlightServerBase::Serve() {
 
 int FlightServerBase::GotSignal() const { return impl_->got_signal_; }
 
-void FlightServerBase::Shutdown() {
-  DCHECK(impl_->server_);
+Status FlightServerBase::Shutdown() {
+  auto server = impl_->server_.get();
+  if (!server) {
+    return Status::Invalid("Shutdown() on uninitialized FlightServerBase");
+  }
   impl_->server_->Shutdown();
+  return Status::OK();
 }
 
 Status FlightServerBase::ListFlights(const ServerCallContext& context,

cpp/src/arrow/flight/server.h

@@ -142,7 +142,7 @@ class ARROW_FLIGHT_EXPORT FlightServerBase {
   /// thread while Serve() blocks.
   ///
   /// TODO(wesm): Shutdown with deadline
-  void Shutdown();
+  Status Shutdown();
 
   // Implement these methods to create your own server. The default
   // implementations will return a not-implemented result to the client

cpp/src/arrow/flight/test-util.cc

@@ -142,7 +142,7 @@ Status InProcessTestServer::Start() {
 }
 
 void InProcessTestServer::Stop() {
-  server_->Shutdown();
+  ARROW_CHECK_OK(server_->Shutdown());
   thread_.join();
 }
 

python/examples/flight/server.py

@@ -122,8 +122,9 @@ def main():
             kwargs["tls_private_key"] = key_file.read()
 
     location = "{}://0.0.0.0:{}".format(scheme, args.port)
+    server.init(location, **kwargs)
     print("Serving on", location)
-    server.run(location, **kwargs)
+    server.run()
 
 
 if __name__ == '__main__':

python/pyarrow/_flight.pyx

@@ -1305,8 +1305,8 @@ cdef class FlightServerBase:
     cdef:
         unique_ptr[PyFlightServer] server
 
-    def run(self, location, auth_handler=None, tls_certificates=None):
-        """Start this server.
+    def init(self, location, auth_handler=None, tls_certificates=None):
+        """Initialize this server.
 
         Parameters
         ----------
@@ -1348,7 +1348,18 @@ cdef class FlightServerBase:
         self.server.reset(c_server)
         with nogil:
             check_status(c_server.Init(deref(c_options)))
-            check_status(c_server.ServeWithSignals())
+
+    def run(self):
+        """
+        Start serving.  This method only returns if shutdown() is called
+        or a signal a received.
+
+        You must have called init() first.
+        """
+        if self.server.get() == nullptr:
+            raise ValueError("run() on uninitialized FlightServerBase")
+        with nogil:
+            check_status(self.server.get().ServeWithSignals())
 
     def list_flights(self, context, criteria):
         raise NotImplementedError
@@ -1381,6 +1392,7 @@ cdef class FlightServerBase:
         # complete. Holding the GIL means Python-implemented Flight
         # methods will never get to run, so this will hang
         # indefinitely.
+        if self.server.get() == nullptr:
+            raise ValueError("shutdown() on uninitialized FlightServerBase")
         with nogil:
-            if self.server.get() != NULL:
-                self.server.get().Shutdown()
+            check_status(self.server.get().Shutdown())

python/pyarrow/includes/libarrow_flight.pxd

@@ -281,7 +281,7 @@ cdef extern from "arrow/python/flight.h" namespace "arrow::py::flight" nogil:
 
         CStatus Init(CFlightServerOptions& options)
         CStatus ServeWithSignals() except *
-        void Shutdown()
+        CStatus Shutdown()
 
     cdef cppclass PyServerAuthHandler\
             " arrow::py::flight::PyServerAuthHandler"(CServerAuthHandler):

python/pyarrow/tests/test_flight.py

@@ -378,13 +378,14 @@ def flight_server(server_base, *args, **kwargs):
 
     ctor_kwargs = kwargs
     server_instance = server_base(*args, **ctor_kwargs)
+    # The server instance needs to be initialized before shutdown()
+    # can be called
+    server_instance.init(location,
+                         auth_handler=auth_handler,
+                         tls_certificates=tls_certificates)
 
     def _server_thread():
-        server_instance.run(
-            location,
-            auth_handler=auth_handler,
-            tls_certificates=tls_certificates,
-        )
+        server_instance.run()
 
     thread = threading.Thread(target=_server_thread, daemon=True)
     thread.start()
@@ -632,7 +633,7 @@ def test_location_invalid():
 
     server = ConstantFlightServer()
     with pytest.raises(pa.ArrowInvalid, match=".*Cannot parse URI:.*"):
-        server.run("%")
+        server.init("%")
 
 
 @pytest.mark.slow
@@ -654,9 +655,6 @@ def test_tls_fails():
             client.do_get(flight.Ticket(b'ints'))
 
 
-# ARROW-5930: Currently fails in Travis with SIGSEGV when tearing down
-# server
-@pytest.mark.slow
 @pytest.mark.requires_testing_data
 def test_tls_do_get():
     """Try a simple do_get call over TLS."""
@@ -673,9 +671,6 @@ def test_tls_do_get():
         assert data.equals(table)
 
 
-# ARROW-5930: Currently fails in Travis with SIGSEGV when tearing down
-# server
-@pytest.mark.slow
 @pytest.mark.requires_testing_data
 def test_tls_override_hostname():
     """Check that incorrectly overriding the hostname fails."""