Bug #1676522

src/calibre/devices/smart_device_app/driver.py

@@ -65,6 +65,14 @@ class ConnectionListener(Thread):
     def stop(self):
         self.keep_running = False
 
+    def _close_socket(self, the_socket):
+        try:
+            the_socket.shutdown(socket.SHUT_RDWR)
+        except:
+            # the shutdown can fail if the socket isn't fully connected. Ignore it
+            pass
+        the_socket.close()
+
     def run(self):
         device_socket = None
         get_all_ips(reinitialize=True)
@@ -141,7 +149,7 @@ class ConnectionListener(Thread):
                         try:
                             self.driver.connection_queue.put_nowait(device_socket)
                         except Queue.Full:
-                            device_socket.close()
+                            self._close_socket(device_socket)
                             device_socket = None
                             self.driver._debug('driver is not answering')
 
@@ -150,7 +158,7 @@ class ConnectionListener(Thread):
                     except socket.error:
                         x = sys.exc_info()[1]
                         self.driver._debug('unexpected socket exception', x.args[0])
-                        device_socket.close()
+                        self._close_socket(device_socket)
                         device_socket = None
 #                        raise
 
@@ -363,6 +371,9 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin):
         self.debug_time = time.time()
         self.is_connected = False
 
+    # Don't call this method from the GUI unless you are sure that there is no
+    # network traffic in progress. Otherwise the gui might hang waiting for the
+    # network timeout
     def _debug(self, *args):
         # manual synchronization so we don't lose the calling method name
         import inspect
@@ -868,10 +879,20 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin):
                 self.device_book_cache[key]['book'] = new_book
                 self.device_book_cache[key]['last_used'] = now()
 
+    # Force close a socket. The shutdown permits the close even if data transfer
+    # is in progress
+    def _close_socket(self, the_socket):
+        try:
+            the_socket.shutdown(socket.SHUT_RDWR)
+        except:
+            # the shutdown can fail if the socket isn't fully connected. Ignore it
+            pass
+        the_socket.close()
+
     def _close_device_socket(self):
         if self.device_socket is not None:
             try:
-                self.device_socket.close()
+                self._close_socket(self.device_socket)
             except:
                 pass
             self.device_socket = None
@@ -896,11 +917,11 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin):
         return port
 
     def _close_listen_socket(self):
-        self.listen_socket.close()
+        self._close_socket(self.listen_socket)
         self.listen_socket = None
         self.is_connected = False
         if getattr(self, 'broadcast_socket', None) is not None:
-            self.broadcast_socket.close()
+            self._close_socket(self.broadcast_socket)
             self.broadcast_socket = None
 
     def _read_file_metadata(self, temp_file_name):
@@ -1806,11 +1827,15 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin):
         self.listen_socket = None
         self.is_connected = False
 
-    @synchronous('sync_lock')
-    def startup_on_demand(self):
+    def _startup_on_demand(self):
         if getattr(self, 'listen_socket', None) is not None:
             # we are already running
             return
+
+        message = None
+        # The driver is not running so must be started. It needs to protect itself
+        # from access by the device thread before it is fully setup. Thus the lock.
+        with self.sync_lock:
             if len(self.opcodes) != len(self.reverse_opcodes):
                 self._debug(self.opcodes, self.reverse_opcodes)
             self.is_connected = False
@@ -1832,7 +1857,6 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin):
             self.have_sent_future_dated_book_message = False
             self.now = None
 
-        message = None
             compression_quality_ok = True
             try:
                 cq = int(self.settings().extra_customization[self.OPT_COMPRESSION_QUALITY])
@@ -1926,7 +1950,7 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin):
                         break
 
                 if port == 0:
-                self.broadcast_socket.close()
+                    self._close_socket(self.broadcast_socket)
                     self.broadcast_socket = None
                     message = 'attaching port to broadcast socket failed. This is not fatal.'
                     self._debug(message)
@@ -1934,12 +1958,15 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin):
             self.connection_queue = Queue.Queue(1)
             self.connection_listener = ConnectionListener(self)
             self.connection_listener.start()
-
         return message
 
-    @synchronous('sync_lock')
-    def shutdown(self):
+    def _shutdown(self):
+        # Force close any socket open by a device. This will cause any IO on the
+        # socket to fail, eventually releasing the transaction lock.
         self._close_device_socket()
+
+        # Now lockup so we can shutdown the control socket and unpublish mDNS
+        with self.sync_lock:
             if getattr(self, 'listen_socket', None) is not None:
                 self.connection_listener.stop()
                 try:
@@ -1950,18 +1977,17 @@ class SMART_DEVICE_APP(DeviceConfig, DevicePlugin):
                     traceback.print_exc()
                 self._close_listen_socket()
 
-    # Methods for dynamic control
+    # Methods for dynamic control. Do not call _debug in these methods, as it
+    # uses the sync lock.
 
     def is_dynamically_controllable(self):
         return 'smartdevice'
 
-    @synchronous('sync_lock')
     def start_plugin(self):
-        return self.startup_on_demand()
+        return self._startup_on_demand()
 
-    @synchronous('sync_lock')
     def stop_plugin(self):
-        self.shutdown()
+        self._shutdown()
 
     def get_option(self, opt_string, default=None):
         opt = self.OPTNAME_TO_NUMBER_MAP.get(opt_string)