fix: Make sure ports are closed properly after the cleanup

lib/device-connections-factory.js

@@ -11,7 +11,7 @@ class iProxy {
     this.deviceport = parseInt(deviceport, 10);
     this.udid = udid;
     this.serverSocket = null;
-    this.log = logger.getLogger(`iProxy@${_.truncate(udid, {length: 8})}`);
+    this.log = logger.getLogger(`iProxy@${udid.substring(0, 8)}`);
   }
 
   async start () {
@@ -21,15 +21,15 @@ class iProxy {
     this.serverSocket = net.createServer(async (connection) => {
       try {
         const socket = await utilities.connectPort(this.udid, this.deviceport);
-        socket.on('close', connection.destroy);
+        socket.on('close', () => connection.end());
         socket.on('error', (e) => this.log.error(e));
-        connection.on('close', socket.destroy);
+        connection.on('close', () => socket.end());
         connection.on('error', (e) => this.log.error(e));
         connection.pipe(socket);
         socket.pipe(connection);
       } catch (e) {
-        this.log.warn(e.message);
-        connection.destroy();
+        this.log.warn(e);
+        connection.end();
       }
     });
     const status = new B((resolve, reject) => {
@@ -40,12 +40,25 @@ class iProxy {
     await status;
   }
 
-  quit () {
+  // eslint-disable-next-line require-await
+  async quit () {
     if (!this.serverSocket) {
       return;
     }
+
+    // TODO: Wait until the connection is actually closed
+    // after the corresponding fixes are done in appium-ios-device
+    // module. Without the fixes the close event might be delayed for up to
+    // 30 seconds (see https://github.com/appium/appium-xcuitest-driver/pull/1094#issuecomment-546578765)
+    this.serverSocket.once('close', () => {
+      this.log.info('The connection has been closed');
+      this.serverSocket = null;
+    });
+    this.serverSocket.once('error', (e) => {
+      this.log.error('Failed to close the connection', e);
+      this.serverSocket = null;
+    });
     this.serverSocket.close();
-    this.serverSocket = null;
   }
 }
 
@@ -70,20 +83,18 @@ class DeviceConnectionsFactory {
     return `${util.hasValue(udid) ? udid : ''}${SPLITTER}${util.hasValue(port) ? port : ''}`;
   }
 
-  _releaseProxiedConnections (connectionKeys) {
-    const result = [];
-    connectionKeys
-      .filter((k) => _.has(this._connectionsMapping[k], 'iproxy'))
-      .map((k) => {
-        log.info(`Releasing the listener for '${k}'`);
+  async _releaseProxiedConnections (connectionKeys) {
+    const promises = [];
+    for (const key of connectionKeys.filter((k) => _.has(this._connectionsMapping[k], 'iproxy'))) {
+      promises.push((async () => {
+        log.info(`Releasing the listener for '${key}'`);
         try {
-          this._connectionsMapping[k].iproxy.quit();
-        } catch (err) {
-          log.warn(`Cannot release the listener for '${k}': ${err.message}`);
-        }
-        result.push(k);
-      });
-    return result;
+          await this._connectionsMapping[key].iproxy.quit();
+        } catch (ign) {}
+        return key;
+      })());
+    }
+    return await B.all(promises);
   }
 
   listConnections (udid = null, port = null, strict = false) {
@@ -129,17 +140,19 @@ class DeviceConnectionsFactory {
 
     if (usePortForwarding) {
       let isPortBusy = (await checkPortStatus(port, '127.0.0.1')) === 'open';
-      log.warn(`Port #${port} is busy`);
-      if (isPortBusy && !_.isEmpty(connectionsOnPort)) {
-        log.info('Trying to release the port');
-        for (const key of this._releaseProxiedConnections(connectionsOnPort)) {
-          delete this._connectionsMapping[key];
-        }
-        if ((await checkPortStatus(port, '127.0.0.1')) !== 'open') {
-          log.info(`Port #${port} has been successfully released`);
-          isPortBusy = false;
-        } else {
-          log.warn(`Did not know how to release port #${port}`);
+      if (isPortBusy) {
+        log.warn(`Port #${port} is busy`);
+        if (!_.isEmpty(connectionsOnPort)) {
+          log.info('Trying to release the port');
+          for (const key of await this._releaseProxiedConnections(connectionsOnPort)) {
+            delete this._connectionsMapping[key];
+          }
+          if ((await checkPortStatus(port, '127.0.0.1')) !== 'open') {
+            log.info(`Port #${port} has been successfully released`);
+            isPortBusy = false;
+          } else {
+            log.warn(`Did not know how to release port #${port}`);
+          }
         }
       }
 
@@ -155,7 +168,9 @@ class DeviceConnectionsFactory {
         await iproxy.start();
         this._connectionsMapping[currentKey] = {iproxy};
       } catch (e) {
-        iproxy.quit();
+        try {
+          await iproxy.quit();
+        } catch (ign) {}
         throw e;
       }
     } else {
@@ -164,7 +179,7 @@ class DeviceConnectionsFactory {
     log.info(`Successfully requested the connection for ${currentKey}`);
   }
 
-  releaseConnection (udid = null, port = null) {
+  async releaseConnection (udid = null, port = null) {
     if (!udid && !port) {
       log.warn('Neither device UDID nor local port is set. ' +
         'Did not know how to release the connection');
@@ -178,7 +193,7 @@ class DeviceConnectionsFactory {
       return;
     }
     log.info(`Found cached connections to release: ${JSON.stringify(keys)}`);
-    this._releaseProxiedConnections(keys);
+    await this._releaseProxiedConnections(keys);
     for (const key of keys) {
       delete this._connectionsMapping[key];
     }

lib/driver.js

@@ -608,6 +608,12 @@ class XCUITestDriver extends BaseDriver {
   async deleteSession () {
     await removeAllSessionWebSocketHandlers(this.server, this.sessionId);
 
+    if (this._recentScreenRecorder) {
+      await this._recentScreenRecorder.interrupt(true);
+      await this._recentScreenRecorder.cleanup();
+      this._recentScreenRecorder = null;
+    }
+
     await this.stop();
 
     if (this.opts.clearSystemFiles && this.isAppTemporary) {
@@ -682,7 +688,7 @@ class XCUITestDriver extends BaseDriver {
       }
     }
 
-    DEVICE_CONNECTIONS_FACTORY.releaseConnection(this.opts.udid);
+    await DEVICE_CONNECTIONS_FACTORY.releaseConnection(this.opts.udid);
   }
 
   async executeCommand (cmd, ...args) {

test/unit/device-connections-factory-specs.js

@@ -47,15 +47,15 @@ describe('DeviceConnectionsFactory', function () {
     devConFactory.listConnections(null, 23424).should.eql([]);
   });
 
-  it('should properly release proxied connections', function () {
+  it('should properly release proxied connections', async function () {
     devConFactory._connectionsMapping = {
       'udid:1234': {iproxy: {quit: () => {}}},
       'udid:5678': {},
       'udid4:6545': {iproxy: {quit: () => {}}},
     };
 
-    devConFactory._releaseProxiedConnections(_.keys(devConFactory._connectionsMapping))
-      .should.eql(['udid:1234', 'udid4:6545']);
+    await devConFactory._releaseProxiedConnections(_.keys(devConFactory._connectionsMapping))
+      .should.eventually.eql(['udid:1234', 'udid4:6545']);
   });
 
 });