From 3c3af9c609cd9b321580347eb7ced302fadb228d Mon Sep 17 00:00:00 2001 From: Amjad Masad Date: Thu, 27 Aug 2015 01:52:56 -0700 Subject: [PATCH] [react-packager] Fix races Summary: A few potential races to fix: 1. Multiple clients maybe racing to delete a zombie socket 2. Servers who should die because other servers are already listening are taking the socket with them (move `process.on('exit'` code to after the server is listening 3. Servers which are redundant should immediatly die --- react-packager/src/SocketInterface/SocketServer.js | 10 +++++++--- react-packager/src/SocketInterface/index.js | 8 +++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/react-packager/src/SocketInterface/SocketServer.js b/react-packager/src/SocketInterface/SocketServer.js index f6faf836..2e91aaba 100644 --- a/react-packager/src/SocketInterface/SocketServer.js +++ b/react-packager/src/SocketInterface/SocketServer.js @@ -32,6 +32,7 @@ class SocketServer { options ); resolve(this); + process.on('exit', () => fs.unlinkSync(sockPath)); }); }); this._server.on('connection', (sock) => this._handleConnection(sock)); @@ -41,8 +42,6 @@ class SocketServer { this._packagerServer = new Server(options); this._jobs = 0; this._dieEventually(); - - process.on('exit', () => fs.unlinkSync(sockPath)); } onReady() { @@ -138,12 +137,17 @@ class SocketServer { process.send({ type: 'createdServer' }); }, error => { - debug('error creating server', error.code); if (error.code === 'EADDRINUSE') { // Server already listening, this may happen if multiple // clients where started in quick succussion (buck). process.send({ type: 'createdServer' }); + + // Kill this server because some other server with the same + // config and socket already started. + debug('server already started'); + setImmediate(() => process.exit()); } else { + debug('error creating server', error.code); throw error; } } diff --git a/react-packager/src/SocketInterface/index.js b/react-packager/src/SocketInterface/index.js index aca35540..19b39bf0 100644 --- a/react-packager/src/SocketInterface/index.js +++ b/react-packager/src/SocketInterface/index.js @@ -13,6 +13,7 @@ const SocketClient = require('./SocketClient'); const SocketServer = require('./SocketServer'); const _ = require('underscore'); const crypto = require('crypto'); +const debug = require('debug')('ReactPackager:SocketInterface'); const fs = require('fs'); const net = require('net'); const path = require('path'); @@ -45,7 +46,12 @@ const SocketInterface = { resolve(SocketClient.create(sockPath)); }); sock.on('error', (e) => { - fs.unlinkSync(sockPath); + try { + debug('deleting socket for not responding', sockPath); + fs.unlinkSync(sockPath); + } catch (err) { + // Another client might have deleted it first. + } createServer(resolve, reject, options, sockPath); }); } else {