From d7b9f64b02a7fc3b6d28f1cb0733c95139a2bdf9 Mon Sep 17 00:00:00 2001 From: booo Date: Fri, 6 Jul 2012 14:08:03 +0200 Subject: [PATCH 1/7] remove unnecessary comments --- src/binding.cc | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/binding.cc b/src/binding.cc index 5253181b..54e4b4e0 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -243,9 +243,7 @@ public: connecting_ = false; TRACE("Initializing ev watchers"); - //ev_init(&read_watcher_, io_event); read_watcher_.data = this; - //ev_init(&write_watcher_, io_event); write_watcher_.data = this; } @@ -353,14 +351,10 @@ protected: PQsetNoticeProcessor(connection_, NoticeReceiver, this); + TRACE("Setting watchers to socket"); uv_poll_init(uv_default_loop(), &read_watcher_, fd); uv_poll_init(uv_default_loop(), &write_watcher_, fd); - TRACE("Setting watchers to socket"); - //uv_poll_start(uv_poll_t* handle, int events, uv_poll_cb cb) - //ev_io_set(&read_watcher_, fd, EV_READ); - //ev_io_set(&write_watcher_, fd, EV_WRITE); - connecting_ = true; StartWrite(); @@ -399,7 +393,7 @@ protected: TRACE("revents & EV_READ"); if(PQconsumeInput(connection_) == 0) { End(); - EmitLastError(); + EmitLastError(); LOG("Something happened, consume input is 0"); return; } @@ -623,7 +617,6 @@ private: void StopWrite() { TRACE("Stoping write watcher"); - //ev_io_stop(EV_DEFAULT_ &write_watcher_); uv_poll_stop(&write_watcher_); } @@ -631,20 +624,17 @@ private: { TRACE("Starting write watcher"); uv_poll_start(&write_watcher_, UV_WRITABLE, io_event); - //ev_io_start(EV_DEFAULT_ &write_watcher_); } void StopRead() { TRACE("Stoping read watcher"); - //ev_io_stop(EV_DEFAULT_ &read_watcher_); uv_poll_stop(&read_watcher_); } void StartRead() { TRACE("Starting read watcher"); - //ev_io_start(EV_DEFAULT_ &read_watcher_); uv_poll_start(&read_watcher_, UV_READABLE, io_event); } //Converts a v8 array to an array of cstrings From 125075b79668f493244e4e05d022773fc363356c Mon Sep 17 00:00:00 2001 From: booo Date: Fri, 6 Jul 2012 14:15:51 +0200 Subject: [PATCH 2/7] call uv_poll_start on Flush() --- src/binding.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/binding.cc b/src/binding.cc index 54e4b4e0..62f0b2ed 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -306,7 +306,7 @@ protected: { if(PQflush(connection_) == 1) { TRACE("Flushing"); - //ev_io_start(EV_DEFAULT_ &write_watcher_); + uv_poll_start(&write_watcher_, UV_WRITABLE, io_event); } } From 60130a933d8be97d0d6482aabc5e1ca5395855cc Mon Sep 17 00:00:00 2001 From: booo Date: Fri, 6 Jul 2012 14:22:59 +0200 Subject: [PATCH 3/7] check for CONNECTION_BAD status before calling PQsetnonblocking --- src/binding.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/binding.cc b/src/binding.cc index 62f0b2ed..1e5ffc0f 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -329,11 +329,6 @@ protected: LOG("Connection couldn't be created"); } - if (PQsetnonblocking(connection_, 1) == -1) { - LOG("Unable to set connection to non-blocking"); - return false; - } - ConnStatusType status = PQstatus(connection_); if(CONNECTION_BAD == status) { @@ -341,6 +336,11 @@ protected: return false; } + if (PQsetnonblocking(connection_, 1) == -1) { + LOG("Unable to set connection to non-blocking"); + return false; + } + int fd = PQsocket(connection_); if(fd < 0) { LOG("socket fd was negative. error"); From 3680b5931b680a5c55ffd424dbf6ba8a96d3455e Mon Sep 17 00:00:00 2001 From: booo Date: Fri, 6 Jul 2012 14:25:07 +0200 Subject: [PATCH 4/7] remove unnecessary LOG call; we emit a proper error later in the code --- src/binding.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/binding.cc b/src/binding.cc index 1e5ffc0f..f5877b6b 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -332,7 +332,6 @@ protected: ConnStatusType status = PQstatus(connection_); if(CONNECTION_BAD == status) { - LOG("Bad connection status"); return false; } From 5329e5e17eedc8339de3c016f5fb90103ad3691f Mon Sep 17 00:00:00 2001 From: booo Date: Fri, 6 Jul 2012 15:16:04 +0200 Subject: [PATCH 5/7] check if the uv_poll_t struct is initialized before calling uv_poll_stop Before this fix a call to End after an error in Connect() would result in a segfault. https://github.com/brianc/node-postgres/issues/136#issuecomment-6774321 --- src/binding.cc | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/binding.cc b/src/binding.cc index f5877b6b..f2ceefc4 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -234,13 +234,14 @@ public: uv_poll_t read_watcher_; uv_poll_t write_watcher_; - PGconn *connection_; bool connecting_; + bool ioInitialized_; Connection () : ObjectWrap () { connection_ = NULL; connecting_ = false; + ioInitialized_ = false; TRACE("Initializing ev watchers"); read_watcher_.data = this; @@ -354,6 +355,8 @@ protected: uv_poll_init(uv_default_loop(), &read_watcher_, fd); uv_poll_init(uv_default_loop(), &write_watcher_, fd); + ioInitialized_ = true; + connecting_ = true; StartWrite(); @@ -616,7 +619,9 @@ private: void StopWrite() { TRACE("Stoping write watcher"); - uv_poll_stop(&write_watcher_); + if(ioInitialized_) { + uv_poll_stop(&write_watcher_); + } } void StartWrite() @@ -628,7 +633,9 @@ private: void StopRead() { TRACE("Stoping read watcher"); - uv_poll_stop(&read_watcher_); + if(ioInitialized_) { + uv_poll_stop(&read_watcher_); + } } void StartRead() From 17e8287bdd1967eeab5ca28d797ce3f994a2b32b Mon Sep 17 00:00:00 2001 From: booo Date: Fri, 6 Jul 2012 15:55:10 +0200 Subject: [PATCH 6/7] call nativeConnect after defining the error handler/callback --- lib/native/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/native/index.js b/lib/native/index.js index 423bbd38..bf2aa0f8 100644 --- a/lib/native/index.js +++ b/lib/native/index.js @@ -30,7 +30,6 @@ p.connect = function(cb) { if(err) { return cb ? cb(err) : self.emit('error', err); } - nativeConnect.call(self, conString); if(cb) { var errCallback; var connectCallback = function() { @@ -46,6 +45,7 @@ p.connect = function(cb) { self.once('connect', connectCallback); self.once('error', errCallback); } + nativeConnect.call(self, conString); }) } From ccc3f81dfa87d007728fdfef65b43c945776b25a Mon Sep 17 00:00:00 2001 From: booo Date: Fri, 6 Jul 2012 15:59:38 +0200 Subject: [PATCH 7/7] enable some useful test cases again --- test/integration/client/error-handling-tests.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/integration/client/error-handling-tests.js b/test/integration/client/error-handling-tests.js index ab6ed99b..a02982ad 100644 --- a/test/integration/client/error-handling-tests.js +++ b/test/integration/client/error-handling-tests.js @@ -80,7 +80,6 @@ test('error handling', function(){ }); test('non-query error', function() { - return false; var client = new Client({ user:'asldkfjsadlfkj' @@ -90,7 +89,6 @@ test('error handling', function(){ }); test('non-query error with callback', function() { - return false; var client = new Client({ user:'asldkfjsadlfkj' }); @@ -117,7 +115,6 @@ test('non-error calls supplied callback', function() { }); test('when connecting to invalid host', function() { - return false; var client = new Client({ user: 'brian', password: '1234', @@ -128,7 +125,6 @@ test('when connecting to invalid host', function() { }); test('when connecting to invalid host with callback', function() { - return false; var client = new Client({ user: 'brian', password: '1234',