Use flock() instead of UUCP-style locking for serial devices

.codespellignorelines

@@ -177,3 +177,4 @@ import java.nio.ByteOrder;
         byte[] header = ByteBuffer.allocate(4).order(ByteOrder.nativeOrder()).putInt(headerContent).array();
         int headerValue = ByteBuffer.wrap(header).order(ByteOrder.nativeOrder()).getInt();
 # This is a very bodgy workaround to the fact that the pip install of the archive doesn't seem to work properly now on Travis
+    "uest\032\036.ola.rpc.STREAMING_NO_RESPONSEB\006\200\001"

common/io/Serial.cpp

@@ -58,8 +58,7 @@ using std::string;
 
 namespace {
 
-#ifdef UUCP_LOCKING
-string GetLockFile(const string &path) {
+string GetUUCPLockFile(const string &path) {
   const string base_name = ola::file::FilenameFromPath(path);
   return ola::file::JoinPaths(UUCP_LOCK_DIR, "LCK.." + base_name);
 }
@@ -108,14 +107,13 @@ bool ProcessExists(pid_t pid) {
 #endif  // _WIN32
 }
 
-bool RemoveLockFile(const string &lock_file) {
+bool RemoveUUCPLockFile(const string &lock_file) {
   if (unlink(lock_file.c_str())) {
     OLA_WARN << "Failed to remove UUCP lock file: " << lock_file;
     return false;
   }
   return true;
 }
-#endif  // UUCP_LOCKING
 
 }  // namespace
 
@@ -143,7 +141,6 @@ bool UIntToSpeedT(uint32_t value, speed_t *output) {
   return false;
 }
 
-#ifdef HAVE_FLOCK
 bool OpenAndFlock(const std::string &path, int oflag, int *fd) {
   // First, check if the path exists, there's no point trying to open it if not
   if (!FileExists(path)) {
@@ -157,11 +154,16 @@ bool OpenAndFlock(const std::string &path, int oflag, int *fd) {
     return false;
   }
 
+#ifdef HAVE_FLOCK
   if (flock(*fd, LOCK_EX | LOCK_NB) == -1) {
     OLA_INFO << "Failed to flock() device " << path;
     close(*fd);
     return false;
   }
+#else  // HAVE_FLOCK
+  close(*fd);
+  return false;
+#endif  // HAVE_FLOCK
 
 #if HAVE_SYS_IOCTL_H
   // As a final safety mechanism, use ioctl(TIOCEXCL) if available to prevent
@@ -172,12 +174,12 @@ bool OpenAndFlock(const std::string &path, int oflag, int *fd) {
     return false;
   }
 #endif  // HAVE_SYS_IOCTL_H
+
+  OLA_INFO << "Locked " << path << " using flock()";
   return true;
 }
-#endif // HAVE_FLOCK
 
 
-#ifdef UUCP_LOCKING
 bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) {
   // This is rather tricky since there is no real convention for LCK files.
   // If it was only a single process doing the locking we could use fnctl as
@@ -191,7 +193,7 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) {
   }
 
   // Second, clean up a stale lockfile.
-  const string lock_file = GetLockFile(path);
+  const string lock_file = GetUUCPLockFile(path);
   OLA_DEBUG << "Checking for " << lock_file;
   pid_t locked_pid;
   if (!GetPidFromFile(lock_file, &locked_pid)) {
@@ -210,7 +212,7 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) {
     }
     // There is a race between the read & the unlink here. I'm not convinced it
     // can be solved.
-    if (!RemoveLockFile(lock_file)) {
+    if (!RemoveUUCPLockFile(lock_file)) {
       OLA_INFO << "Device " << path << " was locked by PID " << locked_pid
                << " which is no longer active, however failed to remove stale "
                << "lock file";
@@ -244,15 +246,15 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) {
   close(lock_fd);
   if (r != pid_file_contents.size()) {
     OLA_WARN << "Failed to write complete LCK file: " << lock_file;
-    RemoveLockFile(lock_file);
+    RemoveUUCPLockFile(lock_file);
     return false;
   }
 
   // Now try to open the serial device.
   if (!TryOpen(path, oflag, fd)) {
     OLA_DEBUG << "Failed to open device " << path << " despite having the "
               << "lock file";
-    RemoveLockFile(lock_file);
+    RemoveUUCPLockFile(lock_file);
     return false;
   }
 
@@ -262,18 +264,16 @@ bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd) {
   if (ioctl(*fd, TIOCEXCL) == -1) {
     OLA_WARN << "TIOCEXCL " << path << " failed: " << strerror(errno);
     close(*fd);
-    RemoveLockFile(lock_file);
+    RemoveUUCPLockFile(lock_file);
     return false;
   }
 #endif  // HAVE_SYS_IOCTL_H
   return true;
 }
-#endif // UUCP_LOCKING
 
 
 void ReleaseUUCPLock(const std::string &path) {
-#ifdef UUCP_LOCKING
-  const string lock_file = GetLockFile(path);
+  const string lock_file = GetUUCPLockFile(path);
 
   pid_t locked_pid;
   if (!GetPidFromFile(lock_file, &locked_pid)) {
@@ -282,20 +282,26 @@ void ReleaseUUCPLock(const std::string &path) {
 
   pid_t our_pid = getpid();
   if (our_pid == locked_pid) {
-    if (RemoveLockFile(lock_file)) {
+    if (RemoveUUCPLockFile(lock_file)) {
       OLA_INFO << "Released " << lock_file;
     }
   }
-#endif  /* else no action needed */
 }
 
-bool AcquireLockAndOpen(const std::string &path, int oflag, int *fd) {
+bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd) {
 #ifdef UUCP_LOCKING
-	return AcquireUUCPLockAndOpen(path, oflag, fd);
+  return AcquireUUCPLockAndOpen(path, oflag, fd);
 #else
-	return OpenAndFlock(path, oflag, fd);
+  return OpenAndFlock(path, oflag, fd);
 #endif
 }
 
+void ReleaseSerialPortLock(const std::string &path) {
+#ifdef UUCP_LOCKING
+  ReleaseUUCPLock(path);
+#endif  // UUCP_LOCKING
+}
+
+
 }  // namespace io
 }  // namespace ola

include/ola/io/Serial.h

@@ -59,6 +59,31 @@ typedef enum {
  */
 bool UIntToSpeedT(uint32_t value, speed_t *output);
 
+/**
+ * @brief Try to open the path, respecting UUCP locking.
+ * @param path the path to open
+ * @param oflag flags passed to open
+ * @param[out] fd a pointer to the fd which is returned.
+ * @returns true if the open succeeded, false otherwise.
+ *
+ * This fails-fast, it we can't get the lock immediately, we'll return false.
+ *
+ * @deprecated Use AcquireLockAndOpenSerialPort() instead.
+ * @see ReleaseUUCPLock()
+ */
+bool AcquireUUCPLockAndOpen(const std::string &path, int oflag, int *fd);
+
+/**
+ * @brief Remove a UUCP lock file for the device.
+ * @param path The path to unlock.
+ *
+ * The lock is only removed if the PID matches.
+ *
+ * @deprecated Use ReleaseSerialPortLock() instead.
+ * @see AcquireUUCPLockAndOpen()
+ */
+void ReleaseUUCPLock(const std::string &path);
+
 /**
  * @brief Try to open the path and obtain a lock to control access
  * @param path the path to open
@@ -70,19 +95,24 @@ bool UIntToSpeedT(uint32_t value, speed_t *output);
  * (the default) or UUCP locking.  See: ./configure --enable-uucp-locking.
  *
  * This fails-fast, it we can't get the lock immediately, we'll return false.
+ *
+ * @see ReleaseSerialPortLock()
  */
-bool AcquireLockAndOpen(const std::string &path, int oflag, int *fd);
+bool AcquireLockAndOpenSerialPort(const std::string &path, int oflag, int *fd);
 
 /**
- * @brief Remove a UUCP lock file for the device.
+ * @brief Release a lock for the serial port
  * @param path The path to unlock.
  *
- * The lock is only removed if the PID matches.
+ * If UUCP locking was used (see AcquireLockAndOpenSerialPort), the lockfile
+ * will be removed (but only if the PID matches).
  *
- * Does nothing if UUCP locking is not in use
- * (see ./configure --enable-uucp-locking).
+ * Does nothing if flock() was used (see ./configure --enable-uucp-locking).
+ *
+ * @see AcquireLockAndOpenSerialPort()
  */
-void ReleaseUUCPLock(const std::string &path);
+void ReleaseSerialPortLock(const std::string &path);
+
 }  // namespace io
 }  // namespace ola
 #endif  // INCLUDE_OLA_IO_SERIAL_H_

plugins/stageprofi/StageProfiDetector.cpp

@@ -126,7 +126,7 @@ void StageProfiDetector::ReleaseWidget(const std::string &widget_path) {
   // the map.
   DescriptorMap::iterator iter = m_usb_widgets.find(widget_path);
   if (iter != m_usb_widgets.end()) {
-    ola::io::ReleaseUUCPLock(widget_path);
+    ola::io::ReleaseSerialPortLock(widget_path);
     iter->second = NULL;
     return;
   }
@@ -164,9 +164,9 @@ ConnectedDescriptor* StageProfiDetector::ConnectToUSB(
   struct termios newtio;
 
   int fd;
-  if (!ola::io::AcquireLockAndOpen(widget_path,
-                                   O_RDWR | O_NONBLOCK | O_NOCTTY,
-                                   &fd)) {
+  if (!ola::io::AcquireLockAndOpenSerialPort(widget_path,
+                                             O_RDWR | O_NONBLOCK | O_NOCTTY,
+                                             &fd)) {
     return NULL;
   }
 

plugins/usbpro/BaseUsbProWidget.cpp

@@ -129,7 +129,7 @@ ola::io::ConnectedDescriptor *BaseUsbProWidget::OpenDevice(
     const string &path) {
   struct termios newtio;
   int fd;
-  if (!ola::io::AcquireLockAndOpen(path, O_RDWR | O_NONBLOCK | O_NOCTTY, &fd)) {
+  if (!ola::io::AcquireLockAndOpenSerialPort(path, O_RDWR | O_NONBLOCK | O_NOCTTY, &fd)) {
     return NULL;
   }
 

plugins/usbpro/WidgetDetectorThread.cpp

@@ -420,7 +420,7 @@ void WidgetDetectorThread::FreeDescriptor(ConnectedDescriptor *descriptor) {
   DescriptorInfo &descriptor_info = m_active_descriptors[descriptor];
 
   m_active_paths.erase(descriptor_info.first);
-  io::ReleaseUUCPLock(descriptor_info.first);
+  io::ReleaseSerialPortLock(descriptor_info.first);
   m_active_descriptors.erase(descriptor);
   delete descriptor;
 }