Merge branch 'devel' of https://github.com/arangodb/arangodb into engine-api

CHANGELOG

@@ -1,7 +1,7 @@
 devel
 -----
 
-* AQL Breaking Change in Cluster:
+* AQL breaking change in cluster:
   The SHORTEST_PATH statement using edge-collection names instead
   of a graph name now requires to explicitly name the vertex-collection names 
   within the AQL query in the cluster. It can be done by adding `WITH <name>`
@@ -44,8 +44,9 @@ devel
   advantage that it is more composable, and will also honor any `LIMIT` values
   used in the AQL query.
 
-* added KB, MB, GB prefix for integer parameters, % for integer parameters
+* potential fix for shutdown hangs on OSX
 
+* added KB, MB, GB prefix for integer parameters, % for integer parameters
   with a base value
 
 * added JEMALLOC 4.5.0
@@ -229,6 +230,12 @@ v3.2.alpha1 (2017-02-05)
 
 * generated Foxx services now use swagger tags
 
+v3.1.19 (XXXX-XX-XX)
+--------------------
+
+
+* Fixed a StackOverflow issue in Traversal and ShortestPath. Occured if many (>1000) input
+  values in a row do not return any result. Fixes issue: #2445
 
 v3.1.19 (XXXX-XX-XX)
 --------------------

Documentation/Books/Manual/Deployment/README.mdpp

@@ -10,8 +10,10 @@ or using Docker containers.
 
 - [Single instance](Single.md)
 - [Cluster: DC/OS, Apache Mesos and Marathon](Mesos.md)
-- [Cluster: Test setup on a local machine](Local.md)
+- [Cluster: Generic & Docker](ArangoDBStarter.md)
-- [Cluster: Starting processes on different machines](Distributed.md)
+- [Advanced Topics](Advanced.md)
-- [Cluster: Launching an ArangoDB cluster using Docker containers](Docker.md)
+  - [Cluster: Test setup on a local machine](Local.md)
-- [Agency](Agency.md)
+  - [Cluster: Starting processes on different machines](Distributed.md)
+  - [Cluster: Launching an ArangoDB cluster using Docker containers](Docker.md)
+  - [Agency](Agency.md)
 

Documentation/Books/Manual/SUMMARY.md

@@ -123,10 +123,12 @@
 * [Deployment](Deployment/README.md)
   * [Single instance](Deployment/Single.md)
   * [Cluster: Mesos, DC/OS](Deployment/Mesos.md)
-  * [Cluster: Local test](Deployment/Local.md)
+  * [Cluster: Generic & Docker](Deployment/ArangoDBStarter.md)
-  * [Cluster: Processes](Deployment/Distributed.md)
+  * [Advanced Topics](Deployment/Advanced.md)
-  * [Cluster: Docker](Deployment/Docker.md)
+    * [Standalone Agency](Deployment/Agency.md)
-  * [Agency](Deployment/Agency.md)
+    * [Cluster: Local test setups](Deployment/Local.md)
+    * [Cluster: Processes](Deployment/Distributed.md)
+    * [Cluster: Docker](Deployment/Docker.md)
 #
 * [Administration](Administration/README.md)
   * [Web Interface](Administration/WebInterface/README.md)

arangod/Aql/ShortestPathBlock.cpp

@@ -287,83 +287,83 @@ bool ShortestPathBlock::nextPath(AqlItemBlock const* items) {
 AqlItemBlock* ShortestPathBlock::getSome(size_t, size_t atMost) {
   DEBUG_BEGIN_BLOCK();
   traceGetSomeBegin();
-  if (_done) {
+  while (true) {
-    traceGetSomeEnd(nullptr);
+    if (_done) {
-    return nullptr;
-  }
-
-  if (_buffer.empty()) {
-    size_t toFetch = (std::min)(DefaultBatchSize(), atMost);
-    if (!ExecutionBlock::getBlock(toFetch, toFetch)) {
-      _done = true;
       traceGetSomeEnd(nullptr);
       return nullptr;
     }
-    _pos = 0;  // this is in the first block
-  }
 
-  // If we get here, we do have _buffer.front()
+    if (_buffer.empty()) {
-  AqlItemBlock* cur = _buffer.front();
+      size_t toFetch = (std::min)(DefaultBatchSize(), atMost);
-  size_t const curRegs = cur->getNrRegs();
+      if (!ExecutionBlock::getBlock(toFetch, toFetch)) {
+        _done = true;
+        traceGetSomeEnd(nullptr);
+        return nullptr;
+      }
+      _pos = 0;  // this is in the first block
+    }
 
-  // Collect the next path:
+    // If we get here, we do have _buffer.front()
-  if (_posInPath >= _pathLength) {
+    AqlItemBlock* cur = _buffer.front();
-    if (!nextPath(cur)) {
+    size_t const curRegs = cur->getNrRegs();
-      // This input does not have any path. maybe the next one has.
+
-      // we can only return nullptr iff the buffer is empty.
+    // Collect the next path:
+    if (_posInPath >= _pathLength) {
+      if (!nextPath(cur)) {
+        // This input does not have any path. maybe the next one has.
+        // we can only return nullptr iff the buffer is empty.
+        if (++_pos >= cur->size()) {
+          _buffer.pop_front();  // does not throw
+          returnBlock(cur);
+          _pos = 0;
+        }
+        continue;
+      }
+    }
+
+    size_t available = _pathLength - _posInPath;
+    size_t toSend = (std::min)(atMost, available);
+
+    RegisterId nrRegs =
+      getPlanNode()->getRegisterPlan()->nrRegs[getPlanNode()->getDepth()];
+    std::unique_ptr<AqlItemBlock> res(requestBlock(toSend, nrRegs));
+    // automatically freed if we throw
+    TRI_ASSERT(curRegs <= res->getNrRegs());
+
+    // only copy 1st row of registers inherited from previous frame(s)
+    inheritRegisters(cur, res.get(), _pos);
+
+    for (size_t j = 0; j < toSend; j++) {
+      if (usesVertexOutput()) {
+        res->setValue(j, _vertexReg,
+            _path->vertexToAqlValue(_opts->cache(), _posInPath));
+      }
+      if (usesEdgeOutput()) {
+        res->setValue(j, _edgeReg,
+            _path->edgeToAqlValue(_opts->cache(), _posInPath));
+      }
+      if (j > 0) {
+        // re-use already copied aqlvalues
+        res->copyValuesFromFirstRow(j, static_cast<RegisterId>(curRegs));
+      }
+      ++_posInPath;
+    }
+
+    if (_posInPath >= _pathLength) {
+      // Advance read position for next call
       if (++_pos >= cur->size()) {
         _buffer.pop_front();  // does not throw
         returnBlock(cur);
         _pos = 0;
       }
-      auto r = getSome(atMost, atMost);
-      traceGetSomeEnd(r);
-      return r;
     }
+
+    // Clear out registers no longer needed later:
+    clearRegisters(res.get());
+    traceGetSomeEnd(res.get());
+    return res.release();
   }
 
-  size_t available = _pathLength - _posInPath;
-  size_t toSend = (std::min)(atMost, available);
-
-  RegisterId nrRegs =
-      getPlanNode()->getRegisterPlan()->nrRegs[getPlanNode()->getDepth()];
-  std::unique_ptr<AqlItemBlock> res(requestBlock(toSend, nrRegs));
-  // automatically freed if we throw
-  TRI_ASSERT(curRegs <= res->getNrRegs());
-
-  // only copy 1st row of registers inherited from previous frame(s)
-  inheritRegisters(cur, res.get(), _pos);
-
-  for (size_t j = 0; j < toSend; j++) {
-    if (usesVertexOutput()) {
-      res->setValue(j, _vertexReg,
-                    _path->vertexToAqlValue(_opts->cache(), _posInPath));
-    }
-    if (usesEdgeOutput()) {
-      res->setValue(j, _edgeReg,
-                    _path->edgeToAqlValue(_opts->cache(), _posInPath));
-    }
-    if (j > 0) {
-      // re-use already copied aqlvalues
-      res->copyValuesFromFirstRow(j, static_cast<RegisterId>(curRegs));
-    }
-    ++_posInPath;
-  }
-
-  if (_posInPath >= _pathLength) {
-    // Advance read position for next call
-    if (++_pos >= cur->size()) {
-      _buffer.pop_front();  // does not throw
-      returnBlock(cur);
-      _pos = 0;
-    }
-  }
-
-  // Clear out registers no longer needed later:
-  clearRegisters(res.get());
-  traceGetSomeEnd(res.get());
-  return res.release();
-
   // cppcheck-suppress style
   DEBUG_END_BLOCK();
 }

arangod/Aql/TraversalBlock.cpp

@@ -354,100 +354,100 @@ AqlItemBlock* TraversalBlock::getSome(size_t,  // atLeast,
                                       size_t atMost) {
   DEBUG_BEGIN_BLOCK();
   traceGetSomeBegin();
-  if (_done) {
+  while (true) {
-    traceGetSomeEnd(nullptr);
+    if (_done) {
-    return nullptr;
-  }
-
-  if (_buffer.empty()) {
-    size_t toFetch = (std::min)(DefaultBatchSize(), atMost);
-    if (!ExecutionBlock::getBlock(toFetch, toFetch)) {
-      _done = true;
       traceGetSomeEnd(nullptr);
       return nullptr;
     }
-    _pos = 0;  // this is in the first block
-  }
 
-  // If we get here, we do have _buffer.front()
+    if (_buffer.empty()) {
-  AqlItemBlock* cur = _buffer.front();
+      size_t toFetch = (std::min)(DefaultBatchSize(), atMost);
-  size_t const curRegs = cur->getNrRegs();
+      if (!ExecutionBlock::getBlock(toFetch, toFetch)) {
-
+        _done = true;
-  if (_pos == 0) {
+        traceGetSomeEnd(nullptr);
-    // Initial initialization
+        return nullptr;
-    initializePaths(cur, _pos);
-  }
-
-  // Iterate more paths:
-  if (_posInPaths >= _vertices.size()) {
-    if (!morePaths(atMost)) {
-      // This input does not have any more paths. maybe the next one has.
-      // we can only return nullptr iff the buffer is empty.
-      _usedConstant = false; // must reset this variable because otherwise the traverser's start vertex may not be reset properly
-      if (++_pos >= cur->size()) {
-        _buffer.pop_front();  // does not throw
-        returnBlock(cur);
-        _pos = 0;
-      } else {
-        initializePaths(cur, _pos);
       }
-      auto r = getSome(atMost, atMost);
+      _pos = 0;  // this is in the first block
-      traceGetSomeEnd(r);
-      return r;
     }
-  }
 
-  size_t available = _vertices.size() - _posInPaths;
+    // If we get here, we do have _buffer.front()
-  size_t toSend = (std::min)(atMost, available);
+    AqlItemBlock* cur = _buffer.front();
+    size_t const curRegs = cur->getNrRegs();
 
-  RegisterId nrRegs =
+    if (_pos == 0) {
-      getPlanNode()->getRegisterPlan()->nrRegs[getPlanNode()->getDepth()];
+      // Initial initialization
-
+      initializePaths(cur, _pos);
-  std::unique_ptr<AqlItemBlock> res(requestBlock(toSend, nrRegs));
-  // automatically freed if we throw
-  TRI_ASSERT(curRegs <= res->getNrRegs());
-
-  // only copy 1st row of registers inherited from previous frame(s)
-  inheritRegisters(cur, res.get(), _pos);
-
-  for (size_t j = 0; j < toSend; j++) {
-    if (usesVertexOutput()) {
-      res->setValue(j, _vertexReg, _vertices[_posInPaths].clone());
     }
-    if (usesEdgeOutput()) {
-      res->setValue(j, _edgeReg, _edges[_posInPaths].clone());
-    }
-    if (usesPathOutput()) {
-      res->setValue(j, _pathReg, _paths[_posInPaths].clone());
-    }
-    if (j > 0) {
-      // re-use already copied AqlValues
-      res->copyValuesFromFirstRow(j, static_cast<RegisterId>(curRegs));
-    }
-    ++_posInPaths;
-  }
-  // Advance read position:
-  if (_posInPaths >= _vertices.size()) {
-    // we have exhausted our local paths buffer
-    // fetch more paths into our buffer
-    if (!morePaths(atMost)) {
-      // nothing more to read, re-initialize fetching of paths
 
-      _usedConstant = false; // must reset this variable because otherwise the traverser's start vertex may not be reset properly
+    // Iterate more paths:
-      if (++_pos >= cur->size()) {
+    if (_posInPaths >= _vertices.size()) {
-        _buffer.pop_front();  // does not throw
+      if (!morePaths(atMost)) {
-        returnBlock(cur);
+        // This input does not have any more paths. maybe the next one has.
-        _pos = 0;
+        // we can only return nullptr iff the buffer is empty.
-      } else {
+        _usedConstant = false; // must reset this variable because otherwise the traverser's start vertex may not be reset properly
-        initializePaths(cur, _pos);
+        if (++_pos >= cur->size()) {
+          _buffer.pop_front();  // does not throw
+          returnBlock(cur);
+          _pos = 0;
+        } else {
+          initializePaths(cur, _pos);
+        }
+        continue;
       }
     }
-  }
 
-  // Clear out registers no longer needed later:
+    size_t available = _vertices.size() - _posInPaths;
-  clearRegisters(res.get());
+    size_t toSend = (std::min)(atMost, available);
-  traceGetSomeEnd(res.get());
+
-  return res.release();
+    RegisterId nrRegs =
+        getPlanNode()->getRegisterPlan()->nrRegs[getPlanNode()->getDepth()];
+
+    std::unique_ptr<AqlItemBlock> res(requestBlock(toSend, nrRegs));
+    // automatically freed if we throw
+    TRI_ASSERT(curRegs <= res->getNrRegs());
+
+    // only copy 1st row of registers inherited from previous frame(s)
+    inheritRegisters(cur, res.get(), _pos);
+
+    for (size_t j = 0; j < toSend; j++) {
+      if (usesVertexOutput()) {
+        res->setValue(j, _vertexReg, _vertices[_posInPaths].clone());
+      }
+      if (usesEdgeOutput()) {
+        res->setValue(j, _edgeReg, _edges[_posInPaths].clone());
+      }
+      if (usesPathOutput()) {
+        res->setValue(j, _pathReg, _paths[_posInPaths].clone());
+      }
+      if (j > 0) {
+        // re-use already copied AqlValues
+        res->copyValuesFromFirstRow(j, static_cast<RegisterId>(curRegs));
+      }
+      ++_posInPaths;
+    }
+    // Advance read position:
+    if (_posInPaths >= _vertices.size()) {
+      // we have exhausted our local paths buffer
+      // fetch more paths into our buffer
+      if (!morePaths(atMost)) {
+        // nothing more to read, re-initialize fetching of paths
+
+        _usedConstant = false; // must reset this variable because otherwise the traverser's start vertex may not be reset properly
+        if (++_pos >= cur->size()) {
+          _buffer.pop_front();  // does not throw
+          returnBlock(cur);
+          _pos = 0;
+        } else {
+          initializePaths(cur, _pos);
+        }
+      }
+    }
+
+    // Clear out registers no longer needed later:
+    clearRegisters(res.get());
+    traceGetSomeEnd(res.get());
+    return res.release();
+  }
 
   // cppcheck-suppress style
   DEBUG_END_BLOCK();

arangod/Cluster/HeartbeatThread.cpp

@@ -825,7 +825,10 @@ void HeartbeatThread::syncDBServerStatusQuo() {
   _backgroundJobScheduledOrRunning = true;
 
   // the JobGuard is in the operator() of HeartbeatBackgroundJob
-  _ioService->post(HeartbeatBackgroundJob(shared_from_this(), TRI_microtime()));
+  if (!isStopping() && !_ioService->stopped()) {
+    _ioService->
+      post(HeartbeatBackgroundJob(shared_from_this(), TRI_microtime()));
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////

js/server/tests/aql/aql-graph-traverser.js

@@ -1514,6 +1514,22 @@ function complexInternaSuite () {
       assertEqual(found, amount);
     },
 
+    testTailRecursion: function () {
+      // This test is to make sure their is no
+      // inifinite callstack in getSome() API
+      let query = `
+        WITH ${vn}
+        FOR id IN 0..100000
+        FOR v IN OUTBOUND CONCAT('${vn}/foobar', id) ${en}
+        RETURN v
+      `;
+
+      let res = db._query(query);
+      assertEqual(res.count(), 0);
+      // With inifinit callstack in getSome this
+      // test will segfault
+    },
+
   };
 
 }