Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixing race condition between mkdir and event listener registration i…

…n pidfiles and logger plugins.
  • Loading branch information...
commit ab8e34946363faf01b4453cda3df6cb8045344d4 1 parent 2a3e68b
Michael Shapiro authored November 03, 2011
161  lib/plugins/logger.js
@@ -11,7 +11,7 @@
11 11
 
12 12
 var fs = require('fs')
13 13
   , Log = require('log')
14  
-  , mkdir = require('mkdirp').mkdirp;
  14
+  , mkdir = require('utils').mkdirPSync;
15 15
 
16 16
 /**
17 17
  * Enable stdout / stderr logs for both the master
@@ -47,104 +47,103 @@ module.exports = function(dir, level){
47 47
   return function(master){
48 48
     dir = master.resolve(dir || 'logs');
49 49
 
50  
-    mkdir(dir, 0755, function(err){
51  
-      if (err) throw err;
52  
-      // master log
53  
-      var stream = fs.createWriteStream(dir + '/master.log', { flags: 'a' });
54  
-      var log = master.log = new Log(level || Log.INFO, stream);
  50
+    mkdir(dir, 0755);
55 51
 
56  
-      // master events
57  
-      master.on('start', function(){
58  
-        log.info('master started');
59  
-      });
  52
+    // master log
  53
+    var stream = fs.createWriteStream(dir + '/master.log', { flags: 'a' });
  54
+    var log = master.log = new Log(level || Log.INFO, stream);
60 55
 
61  
-      // master is shutting down
62  
-      master.on('closing', function(){
63  
-        log.warning('shutting down master');
64  
-      });
  56
+    // master events
  57
+    master.on('start', function(){
  58
+      log.info('master started');
  59
+    });
65 60
 
66  
-      // master has closed and performed cleanup
67  
-      master.on('close', function(){
68  
-        log.info('shutdown complete');
69  
-      });
  61
+    // master is shutting down
  62
+    master.on('closing', function(){
  63
+      log.warning('shutting down master');
  64
+    });
70 65
 
71  
-      // sending signal to all workers
72  
-      master.on('kill', function(sig){
73  
-        log.warning('sent kill(%s) to all workers', sig);
74  
-      });
  66
+    // master has closed and performed cleanup
  67
+    master.on('close', function(){
  68
+      log.info('shutdown complete');
  69
+    });
75 70
 
76  
-      // worker was killed
77  
-      master.on('worker killed', function(worker){
78  
-        if ('restarting' == master.state) return;
79  
-        log.error('worker %s died', worker.id);
80  
-      });
  71
+    // sending signal to all workers
  72
+    master.on('kill', function(sig){
  73
+      log.warning('sent kill(%s) to all workers', sig);
  74
+    });
81 75
 
82  
-      // worker exception
83  
-      master.on('worker exception', function(worker, err){
84  
-        log.error('worker %s uncaught exception %s', worker.id, err.message);
85  
-      });
  76
+    // worker was killed
  77
+    master.on('worker killed', function(worker){
  78
+      if ('restarting' == master.state) return;
  79
+      log.error('worker %s died', worker.id);
  80
+    });
86 81
 
87  
-      // worker is waiting on connections to be closed
88  
-      master.on('worker waiting', function(worker, connections){
89  
-        log.info('worker %s waiting on %s connections', worker.id, connections);
90  
-      });
  82
+    // worker exception
  83
+    master.on('worker exception', function(worker, err){
  84
+      log.error('worker %s uncaught exception %s', worker.id, err.message);
  85
+    });
91 86
 
92  
-      // worker has timed out
93  
-      master.on('worker timeout', function(worker, timeout){
94  
-        log.warning('worker %s timed out after %sms', worker.id, timeout);
95  
-      });
  87
+    // worker is waiting on connections to be closed
  88
+    master.on('worker waiting', function(worker, connections){
  89
+      log.info('worker %s waiting on %s connections', worker.id, connections);
  90
+    });
96 91
 
97  
-      // worker connected to master
98  
-      master.on('worker connected', function(worker){
99  
-        log.debug('worker %s connected', worker.id);
100  
-      });
  92
+    // worker has timed out
  93
+    master.on('worker timeout', function(worker, timeout){
  94
+      log.warning('worker %s timed out after %sms', worker.id, timeout);
  95
+    });
101 96
 
102  
-      // cyclic or immediate restart
103  
-      master.on('cyclic restart', function(){
104  
-        log.warning('cyclic restart detected, restarting in %sms'
105  
-          , master.options['restart timeout']);
106  
-      });
  97
+    // worker connected to master
  98
+    master.on('worker connected', function(worker){
  99
+      log.debug('worker %s connected', worker.id);
  100
+    });
107 101
 
108  
-      // restart requested
109  
-      master.on('restarting', function(){
110  
-        log.info('restart requested');
111  
-      });
  102
+    // cyclic or immediate restart
  103
+    master.on('cyclic restart', function(){
  104
+      log.warning('cyclic restart detected, restarting in %sms'
  105
+        , master.options['restart timeout']);
  106
+    });
112 107
 
113  
-      // restart complete
114  
-      master.on('restart', function(){
115  
-        log.info('restart complete');
116  
-      });
  108
+    // restart requested
  109
+    master.on('restarting', function(){
  110
+      log.info('restart requested');
  111
+    });
  112
+
  113
+    // restart complete
  114
+    master.on('restart', function(){
  115
+      log.info('restart complete');
  116
+    });
117 117
 
118  
-      // repl socket connection established
119  
-      master.on('repl socket', function(sock){
120  
-        var from = sock.remoteAddress
121  
-          ? 'from ' + sock.remoteAddress
122  
-          : '';
123  
-        sock.on('connect', function(){
124  
-          log.info('repl connection %s', from);
125  
-        });
126  
-        sock.on('close', function(){
127  
-          log.info('repl disconnect %s', from);
128  
-        });
  118
+    // repl socket connection established
  119
+    master.on('repl socket', function(sock){
  120
+      var from = sock.remoteAddress
  121
+        ? 'from ' + sock.remoteAddress
  122
+        : '';
  123
+      sock.on('connect', function(){
  124
+        log.info('repl connection %s', from);
129 125
       });
  126
+      sock.on('close', function(){
  127
+        log.info('repl disconnect %s', from);
  128
+      });
  129
+    });
130 130
 
131  
-      // override fds
132  
-      master.customFds = [-1, -1];
  131
+    // override fds
  132
+    master.customFds = [-1, -1];
133 133
 
134  
-      // children
135  
-      master.on('worker', function(worker){
136  
-        var proc = worker.proc;
  134
+    // children
  135
+    master.on('worker', function(worker){
  136
+      var proc = worker.proc;
137 137
 
138  
-        log.info('spawned worker ' + worker.id);
  138
+      log.info('spawned worker ' + worker.id);
139 139
 
140  
-        // worker log streams
141  
-        var access = fs.createWriteStream(dir + '/workers.access.log', { flags: 'a' })
142  
-          , error = fs.createWriteStream(dir + '/workers.error.log', { flags: 'a' });
  140
+      // worker log streams
  141
+      var access = fs.createWriteStream(dir + '/workers.access.log', { flags: 'a' })
  142
+        , error = fs.createWriteStream(dir + '/workers.error.log', { flags: 'a' });
143 143
 
144  
-        // redirect stdout / stderr
145  
-        proc.stdout.pipe(access);
146  
-        proc.stderr.pipe(error);
147  
-      });
  144
+      // redirect stdout / stderr
  145
+      proc.stdout.pipe(access);
  146
+      proc.stderr.pipe(error);
148 147
     });
149 148
   }
150  
-};
  149
+};
32  lib/plugins/pidfiles.js
@@ -10,7 +10,7 @@
10 10
  */
11 11
 
12 12
 var fs = require('fs')
13  
-  , mkdir = require('mkdirp').mkdirp;
  13
+  , mkdir = require('utils').mkdirPSync;
14 14
 
15 15
 /**
16 16
  * Save pidfiles to the given `dir` or `./pids`.
@@ -59,25 +59,23 @@ module.exports = function(dir){
59 59
       });
60 60
     };
61 61
 
62  
-    mkdir(dir, 0755, function(err){
63  
-      if (err) throw err;
  62
+    mkdir(dir, 0755);
64 63
 
65  
-      // save worker pids
66  
-      master.on('worker', function(worker){
67  
-        var path = dir + '/worker.' + worker.id + '.pid';
68  
-        fs.writeFile(path, worker.proc.pid.toString(), 'ascii', function(err){
69  
-          if (err) throw err;
70  
-          master.emit('worker pidfile');
71  
-        });
  64
+    // save worker pids
  65
+    master.on('worker', function(worker){
  66
+      var path = dir + '/worker.' + worker.id + '.pid';
  67
+      fs.writeFile(path, worker.proc.pid.toString(), 'ascii', function(err){
  68
+        if (err) throw err;
  69
+        master.emit('worker pidfile');
72 70
       });
  71
+    });
73 72
 
74  
-      master.on('listening', function(){
75  
-        // save master pid
76  
-        fs.writeFile(dir + '/master.pid', process.pid.toString(), 'ascii', function(err){
77  
-          if (err) throw err; 
78  
-          master.emit('pidfile');
79  
-        });
  73
+    master.on('listening', function(){
  74
+      // save master pid
  75
+      fs.writeFile(dir + '/master.pid', process.pid.toString(), 'ascii', function(err){
  76
+        if (err) throw err; 
  77
+        master.emit('pidfile');
80 78
       });
81 79
     });
82 80
   }
83  
-};
  81
+};
27  lib/utils.js
@@ -5,6 +5,10 @@
5 5
  * MIT Licensed
6 6
  */
7 7
 
  8
+
  9
+var path = require('path')
  10
+  , fs   = require('fs');
  11
+
8 12
 /**
9 13
  * Frame the given `obj`.
10 14
  *
@@ -95,4 +99,25 @@ exports.unshiftListener = function(obj, event, fn){
95 99
   } else {
96 100
     obj._events[event] = [fn, obj._events[event]];
97 101
   }
98  
-};
  102
+};
  103
+
  104
+
  105
+/**
  106
+ * `mkdir -p`, synchronously
  107
+ *
  108
+ * @param {String} dir
  109
+ * @param {String} mode
  110
+ * @api private
  111
+ */
  112
+
  113
+exports.mkdirPSync = function(dir, mode) {
  114
+  var buildingPath = [],
  115
+      components = path.normalize(dir).split('/');
  116
+
  117
+  components.slice(1, components.length).forEach(function(component) {
  118
+    buildingPath = buildingPath.concat(component);
  119
+    var toCreate = '/' + buildingPath.join('/');
  120
+    if (!path.existsSync(toCreate))
  121
+      fs.mkdirSync('/' + buildingPath.join('/'), mode);
  122
+  });
  123
+};

0 notes on commit ab8e349

Tomasz Elendt

why not mkdir = require('mkdirp').mkdirp.sync?

Michael Shapiro

I wasn't aware of that, was it added in a recent version of node? (also, cluster seems to be abandoned, or something)

Please sign in to comment.
Something went wrong with that request. Please try again.