Merge branch 'bugfix/pcicclient_deadlock_on_windows' into 'master'

Bugfix/pcicclient deadlock on windows

See merge request syntron/support/csr/ifm3d/ifm3d!115

CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.5)
 cmake_policy(SET CMP0048 NEW)
 # Manage version number here: Major.Minor.Patch (we don't use "tweak")
-project(IFM3D VERSION 0.20.2 LANGUAGES CXX)
+project(IFM3D VERSION 0.20.3 LANGUAGES CXX)
 set(GIT_PROJECT_NAME "ifm3d")
 
 # Make our cmake functions accessible

ChangeLog.md

@@ -1,3 +1,7 @@
+## Changes between ifm3d 0.20.2 and 0.20.3
+* Bugfixes
+ * Pcicclient deadlock after the timeout occur on windows
+
 ## Changes between ifm3d 0.20.1 and 0.20.2
 * Bugfixes
  * Pcicclient deadlock after the timeout occur.

README.md

@@ -22,11 +22,11 @@ Current Revision
     <th>Notes</th>
   </tr>
   <tr>
-    <td>0.20.2 </td>
+    <td>0.20.3 </td>
     <td>1.6.2114, 1.23.1522, 1.23.1522, 1.23.2848, 1.30.4123, 1.30.5309</td>
     <td>1.0.122, 1.0.126, 1.0.156, 1.1.190</td>
     <td>18.04,20.04</td>
-    <td>Bugfix for pcicclient deadlock on network interruption</td>
+    <td>Resolved pcicclient deadlock after the timeout occur on windows</td>
   </tr>
 </table>
 

doc/swcompat.md

@@ -246,10 +246,17 @@ ifm3d Software Compatibility Matrix
     reload app during config when temporal filter is set</td>
   </tr>
   <tr>
-    <td>0.20.1 </td>
+    <td>0.20.2 </td>
     <td>1.6.2114, 1.23.1522, 1.23.1522, 1.23.2848, 1.30.4123, 1.30.5309</td>
     <td>1.0.122, 1.0.126, 1.0.156, 1.1.190</td>
     <td>18.04,20.04</td>
     <td>Bugfix for pcicclient deadlock on network interruption</td>
   </tr>
+  <tr>
+    <td>0.20.3 </td>
+    <td>1.6.2114, 1.23.1522, 1.23.1522, 1.23.2848, 1.30.4123, 1.30.5309</td>
+    <td>1.0.122, 1.0.126, 1.0.156, 1.1.190</td>
+    <td>18.04,20.04</td>
+    <td>Resolved pcicclient deadlock after the timeout occur on windows</td>
+  </tr>
 </table>

modules/camera/include/ifm3d/camera/err.h

@@ -9,6 +9,7 @@
 #define __IFM3D_CAMERA_ERR_H__
 
 #include <exception>
+#include <string>
 #include <ifm3d/camera/camera_export.h>
 
 // library errors
@@ -97,7 +98,7 @@ namespace ifm3d
      * The ctor simply sets the error value into a local instance variable that
      * may be retrieved with a call to `code()`.
      */
-    error_t(int errnum);
+    error_t(int errnum, const std::string& msg = "");
 
     /**
      * Exception message
@@ -109,13 +110,23 @@ namespace ifm3d
      */
     int code() const noexcept;
 
+    /**
+     * operator()
+     * */
+    void operator()(const int errnum, const std::string& msg = "");
+
   private:
     /**
      * The error code from the sensor/system/library that this exception
      * wraps.
      */
     int errnum_;
 
+    /**
+     * The error message from the user
+     */
+    std::string user_message_;
+
   }; // end: class error_t
 
 } // end: namespace ifm3d

modules/camera/src/libifm3d_camera/err.cpp

@@ -5,6 +5,7 @@
  */
 
 #include <ifm3d/camera/err.h>
+#include <string>
 #include <cstring>
 
 // library errors
@@ -185,7 +186,11 @@ ifm3d::strerror(int errnum)
     }
 }
 
-ifm3d::error_t::error_t(int errnum) : std::exception(), errnum_(errnum) { }
+ifm3d::error_t::error_t(int errnum, const std::string& msg)
+  : std::exception(),
+    errnum_(errnum),
+    user_message_(msg)
+{ }
 
 int
 ifm3d::error_t::code() const noexcept
@@ -196,5 +201,16 @@ ifm3d::error_t::code() const noexcept
 const char*
 ifm3d::error_t::what() const noexcept
 {
+  if (!user_message_.empty())
+    {
+      return this->user_message_.c_str();
+    }
   return ifm3d::strerror(this->code());
 }
+
+void
+ifm3d::error_t::operator()(const int errnum, const std::string& msg)
+{
+  this->errnum_ = errnum;
+  this->user_message_ = msg;
+}

modules/pcicclient/src/libifm3d_pcicclient/pcicclient_impl.hpp

@@ -388,6 +388,16 @@ namespace ifm3d
      */
     std::condition_variable in_cv_;
 
+    /**
+     * Network error
+     */
+    ifm3d::error_t network_error_;
+
+    /**
+     * Network error flag
+     */
+    std::atomic_bool network_interrupted_;
+
   }; // end: class FrameGrabber::Impl
 
 } // end: namespace ifm3d
@@ -415,7 +425,9 @@ ifm3d::PCICClient::Impl::Impl(ifm3d::Camera::Ptr cam,
     in_content_buffer_(),
     in_post_content_buffer_(ifm3d::POST_CONTENT_BUFFER_LENGTH, ' '),
     out_pre_content_buffer_(ifm3d::PRE_CONTENT_BUFFER_LENGTH, ' '),
-    out_post_content_buffer_("\r\n")
+    out_post_content_buffer_("\r\n"),
+    network_error_(0, ""),
+    network_interrupted_(false)
 {
   try
     {
@@ -464,7 +476,6 @@ ifm3d::PCICClient::Impl::~Impl()
       this->Stop();
       this->thread_->join();
     }
-
   VLOG(IFM3D_TRACE) << "FrameGrabber destroyed.";
 }
 
@@ -532,8 +543,9 @@ ifm3d::PCICClient::Impl::Call(
 
   // Wait until sending is complete
   std::unique_lock<std::mutex> out_mutex_lock(this->out_mutex_);
-  this->out_cv_.wait(out_mutex_lock,
-                     [this] { return this->out_completed_.load(); });
+  this->out_cv_.wait(out_mutex_lock, [this] {
+    return (this->out_completed_.load() || this->network_interrupted_.load());
+  });
 
   return callback_id;
 }
@@ -570,6 +582,11 @@ ifm3d::PCICClient::Impl::Call(const std::string& request,
   if (call_thread_ && call_thread_->joinable())
     call_thread_->join();
 
+  if (network_interrupted_.load())
+    {
+      throw network_error_;
+    }
+
   // Check the return value of our PCIC Call
   auto predicate = [&has_result] { return has_result.load(); };
   if (call_output > 0)
@@ -659,6 +676,19 @@ ifm3d::PCICClient::Impl::DoConnect()
                   std::placeholders::_1));
       this->io_service_.run();
     }
+  catch (const ifm3d::error_t& ex)
+    {
+      if (ex.code() != IFM3D_THREAD_INTERRUPTED)
+        {
+          LOG(ERROR) << "Exception: " << ex.what();
+          network_error_(ex.code(), ex.what());
+          network_interrupted_.store(true);
+        }
+      else
+        {
+          LOG(WARNING) << "Exception: " << ex.what();
+        }
+    }
   catch (const std::exception& ex)
     {
       LOG(WARNING) << "Exception: " << ex.what();
@@ -672,7 +702,7 @@ ifm3d::PCICClient::Impl::ConnectHandler(const asio::error_code& ec)
 {
   if (ec)
     {
-      throw ifm3d::error_t(ec.value());
+      throw ifm3d::error_t(ec.value(), ec.message());
     }
   this->DoRead(State::PRE_CONTENT);
 
@@ -683,7 +713,7 @@ ifm3d::PCICClient::Impl::ConnectHandler(const asio::error_code& ec)
     [&, this](const asio::error_code& ec, std::size_t bytes_transferred) {
       if (ec)
         {
-          throw ifm3d::error_t(ec.value());
+          throw ifm3d::error_t(ec.value(), ec.message());
         }
       this->connected_.store(true);
     });
@@ -716,7 +746,7 @@ ifm3d::PCICClient::Impl::ReadHandler(State state,
 {
   if (ec)
     {
-      throw ifm3d::error_t(ec.value());
+      throw ifm3d::error_t(ec.value(), ec.message());
     }
 
   if (bytes_remaining - bytes_transferred > 0)
@@ -829,7 +859,7 @@ ifm3d::PCICClient::Impl::WriteHandler(State state,
 {
   if (ec)
     {
-      throw ifm3d::error_t(ec.value());
+      throw ifm3d::error_t(ec.value(), ec.message());
     }
 
   if (bytes_remaining - bytes_transferred > 0)