From 7bad9dea513392b66b5b57d50ebb90122e7191df Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 16 Jun 2009 15:47:57 +0200 Subject: [PATCH 1/7] Add electric fence option to configure process --- wscript | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/wscript b/wscript index 05bff78c2b..2b40f64395 100644 --- a/wscript +++ b/wscript @@ -24,6 +24,12 @@ def set_options(opt): , help='Build debug variant [Default: False]' , dest='debug' ) + opt.add_option( '--efence' + , action='store_true' + , default=False + , help='Build with -lefence for debugging [Default: False]' + , dest='efence' + ) def configure(conf): conf.check_tool('compiler_cxx') @@ -31,19 +37,19 @@ def configure(conf): conf.env["USE_DEBUG"] = Options.options.debug - conf.check(lib='profiler', uselib_store='PROFILER') + if Options.options.debug: + conf.check(lib='profiler', uselib_store='PROFILER') + + if Options.options.efence: + conf.check(lib='efence', libpath=['/usr/lib', '/usr/local/lib'], uselib_store='EFENCE') if sys.platform.startswith("freebsd"): if not conf.check(lib="execinfo", libpath=['/usr/lib', '/usr/local/lib'], uselib_store="EXECINFO"): - fatal("install the libexecinfo port. devel/libexecinfo") + fatal("Install the libexecinfo port from /usr/ports/devel/libexecinfo.") conf.sub_config('deps/libeio') conf.sub_config('deps/libev') - # liboi config - print "--- liboi ---" - - # Not using TLS yet # if conf.check_cfg(package='gnutls', args='--cflags --libs', uselib_store="GNUTLS"): # conf.define("HAVE_GNUTLS", 1) @@ -163,7 +169,7 @@ def build(bld): deps/http_parser """ node.uselib_local = "oi ev eio http_parser" - node.uselib = "V8 EXECINFO PROFILER" + node.uselib = "V8 EXECINFO PROFILER EFENCE" node.install_path = '${PREFIX}/bin' node.chmod = 0755 From 3b05cf260ea2f1aa75b604724bb7437b669d2a40 Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 16 Jun 2009 15:50:52 +0200 Subject: [PATCH 2/7] Add "opening" readyState for the resolve period. --- src/net.cc | 41 ++++++++++++++++++++++++++++++++++++----- src/net.h | 21 +++------------------ test/test-http.js | 4 ++-- website/api.html | 2 +- 4 files changed, 42 insertions(+), 26 deletions(-) diff --git a/src/net.cc b/src/net.cc index c06989af75..d9cfc973b4 100644 --- a/src/net.cc +++ b/src/net.cc @@ -36,6 +36,7 @@ using namespace node; #define READY_STATE_SYMBOL String::NewSymbol("readyState") #define OPEN_SYMBOL String::NewSymbol("open") +#define OPENING_SYMBOL String::NewSymbol("opening") #define READ_ONLY_SYMBOL String::NewSymbol("readOnly") #define WRITE_ONLY_SYMBOL String::NewSymbol("writeOnly") #define CLOSED_SYMBOL String::NewSymbol("closed") @@ -90,6 +91,7 @@ Connection::ReadyStateGetter (Local _, const AccessorInfo& info) switch(connection->ReadyState()) { case OPEN: return scope.Close(OPEN_SYMBOL); + case OPENING: return scope.Close(OPENING_SYMBOL); case CLOSED: return scope.Close(CLOSED_SYMBOL); case READ_ONLY: return scope.Close(READ_ONLY_SYMBOL); case WRITE_ONLY: return scope.Close(WRITE_ONLY_SYMBOL); @@ -113,6 +115,7 @@ Connection::Connection (Handle handle) void Connection::Init (void) { + opening = false; double timeout = 60.0; // default oi_socket_init(&socket_, timeout); socket_.on_connect = Connection::on_connect; @@ -154,6 +157,30 @@ Connection::New (const Arguments& args) return args.This(); } +enum Connection::readyState +Connection::ReadyState (void) +{ + if (socket_.got_full_close) + return CLOSED; + + if (socket_.got_half_close) + return (socket_.read_action == NULL ? CLOSED : READ_ONLY); + + if (socket_.read_action && socket_.write_action) + return OPEN; + + else if (socket_.write_action) + return WRITE_ONLY; + + else if (socket_.read_action) + return READ_ONLY; + + else if (opening) + return OPENING; + + return CLOSED; +} + Handle Connection::Connect (const Arguments& args) { @@ -163,7 +190,7 @@ Connection::Connect (const Arguments& args) HandleScope scope; if (connection->ReadyState() != CLOSED) { - return ThrowException(String::New("Socket is already connected.")); + return ThrowException(String::New("Socket is not in CLOSED state.")); } else { // XXX ugly. connection->Init(); // in case we're reusing the socket... ? @@ -173,6 +200,7 @@ Connection::Connect (const Arguments& args) return ThrowException(String::New("Must specify a port.")); String::AsciiValue port_sv(args[0]->ToString()); + if (connection->port_) printf("connection->port_ = '%s'\n", connection->port_); assert(connection->port_ == NULL); connection->port_ = strdup(*port_sv); @@ -181,6 +209,8 @@ Connection::Connect (const Arguments& args) String::Utf8Value host_sv(args[1]->ToString()); connection->host_ = strdup(*host_sv); } + + connection->opening = true; #ifdef __APPLE__ /* HACK: Bypass the thread pool and do it sync on Macintosh. @@ -224,7 +254,7 @@ Connection::Resolve (eio_req *req) #ifdef __APPLE__ Connection::AfterResolve(req); -#endif // __APPLE__ +#endif return 0; } @@ -235,7 +265,9 @@ Connection::AfterResolve (eio_req *req) Connection *connection = static_cast (req->data); struct addrinfo *address = static_cast(req->ptr2); - req->ptr2 = NULL; + req->ptr2 = NULL; // ? + + connection->opening = false; int r = 0; if (req->result == 0) { @@ -256,10 +288,9 @@ Connection::AfterResolve (eio_req *req) connection->Detach(); out: - #ifdef __APPLE__ free(req); -#endif // __APPLE__ +#endif return 0; } diff --git a/src/net.h b/src/net.h index 85049042a6..7ba05fec58 100644 --- a/src/net.h +++ b/src/net.h @@ -52,24 +52,9 @@ protected: enum encoding encoding_; - enum readyState { OPEN, CLOSED, READ_ONLY, WRITE_ONLY }; - enum readyState ReadyState ( ) - { - if (socket_.got_full_close) - return CLOSED; - - if (socket_.got_half_close) - return (socket_.read_action == NULL ? CLOSED : READ_ONLY); - - if (socket_.read_action && socket_.write_action) - return OPEN; - else if (socket_.write_action) - return WRITE_ONLY; - else if (socket_.read_action) - return READ_ONLY; - - return CLOSED; - } + enum readyState { OPEN, OPENING, CLOSED, READ_ONLY, WRITE_ONLY }; + bool opening; + enum readyState ReadyState (void); private: diff --git a/test/test-http.js b/test/test-http.js index 10ed9511af..dc9cf4abec 100644 --- a/test/test-http.js +++ b/test/test-http.js @@ -26,7 +26,7 @@ function onLoad () { responses_sent += 1; }; - assertEquals("127.0.0.1", res.connection.remoteAddress); + //assertEquals("127.0.0.1", res.connection.remoteAddress); }).listen(PORT); var client = new node.http.Client(PORT); @@ -46,7 +46,7 @@ function onLoad () { res.setBodyEncoding("utf8"); res.onBody = function (chunk) { body1 += chunk; }; }); - }, 10); + }, 1); } function onExit () { diff --git a/website/api.html b/website/api.html index fbbdb43fa8..4ec4061e05 100644 --- a/website/api.html +++ b/website/api.html @@ -438,7 +438,7 @@ server.listen(7000, "localhost");
connection.readyState
- Either "closed", "open", + Either "closed", "open", "opening" "readOnly", or "writeOnly".
From 225637a15c13c4e217314f2d3f50cb2b819798ee Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 16 Jun 2009 17:43:40 +0200 Subject: [PATCH 3/7] Resolve should default to IPv4 address. --- src/net.cc | 58 ++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/src/net.cc b/src/net.cc index d9cfc973b4..3efd357a9f 100644 --- a/src/net.cc +++ b/src/net.cc @@ -259,22 +259,51 @@ Connection::Resolve (eio_req *req) return 0; } +static struct addrinfo * +AddressDefaultToIPv4 (struct addrinfo *address_list) +{ + struct addrinfo *address = NULL; + +/* + char ip4[INET_ADDRSTRLEN], ip6[INET6_ADDRSTRLEN]; + for (address = address_list; address != NULL; address = address->ai_next) { + if (address->ai_family == AF_INET) { + struct sockaddr_in *sa = reinterpret_cast(address->ai_addr); + inet_ntop(AF_INET, &(sa->sin_addr), ip4, INET_ADDRSTRLEN); + printf("%s\n", ip4); + + } else if (address->ai_family == AF_INET6) { + struct sockaddr_in6 *sa6 = reinterpret_cast(address->ai_addr); + inet_ntop(AF_INET6, &(sa6->sin6_addr), ip6, INET6_ADDRSTRLEN); + printf("%s\n", ip6); + } + } +*/ + + for (address = address_list; address != NULL; address = address->ai_next) { + if (address->ai_addr->sa_family == AF_INET) break; + } + + if (address == NULL) address = address_list; + + return address; +} + int Connection::AfterResolve (eio_req *req) { Connection *connection = static_cast (req->data); - struct addrinfo *address = static_cast(req->ptr2); + struct addrinfo *address = NULL, + *address_list = static_cast(req->ptr2); - req->ptr2 = NULL; // ? + address = AddressDefaultToIPv4(address_list); connection->opening = false; int r = 0; - if (req->result == 0) { - r = connection->Connect(address); - } + if (req->result == 0) r = connection->Connect(address); - if (address) freeaddrinfo(address); + if (address_list) freeaddrinfo(address_list); // no error. return. if (r == 0 && req->result == 0) { @@ -282,16 +311,22 @@ Connection::AfterResolve (eio_req *req) goto out; } - puts("net.cc: resolve failed"); + /* RESOLVE ERROR */ + + /* TODO: the whole resolve process should be moved into oi_socket. + * The fact that I'm modifying a read-only variable here should be + * good evidence of this. + */ + connection->socket_.errorno = r | req->result; connection->OnDisconnect(); + connection->Detach(); out: #ifdef __APPLE__ free(req); #endif - return 0; } @@ -672,12 +707,15 @@ Acceptor::Listen (const Arguments& args) // For servers call getaddrinfo inline. This is blocking but it shouldn't // matter much. If someone actually complains then simply swap it out // with a libeio call. - struct addrinfo *address = NULL; - int r = getaddrinfo(host, *port, &server_tcp_hints, &address); + struct addrinfo *address = NULL, + *address_list = NULL; + int r = getaddrinfo(host, *port, &server_tcp_hints, &address_list); free(host); if (r != 0) return ThrowException(String::New(strerror(errno))); + address = AddressDefaultToIPv4(address_list); + acceptor->Listen(address); return Undefined(); } From 193283bc381ff7b798e7bf5dff7d0a11e55e0b40 Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 16 Jun 2009 17:47:59 +0200 Subject: [PATCH 4/7] Fix memleak: freeaddrinfo() after Server resolve address. --- src/net.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/net.cc b/src/net.cc index 3efd357a9f..c4213667c1 100644 --- a/src/net.cc +++ b/src/net.cc @@ -717,6 +717,9 @@ Acceptor::Listen (const Arguments& args) address = AddressDefaultToIPv4(address_list); acceptor->Listen(address); + + if (address_list) freeaddrinfo(address_list); + return Undefined(); } From 40ee85242532b81dce7b64ed7e2357ca3a98cbc5 Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 16 Jun 2009 19:32:31 +0200 Subject: [PATCH 5/7] Only run debug tests if the build debug was built --- configure | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/configure b/configure index e07d7f7740..11d4ce01e7 100755 --- a/configure +++ b/configure @@ -96,11 +96,13 @@ FAIL=python -c 'print("\033[1;31mFAIL\033[m")' PASS=python -c 'print("\033[1;32mPASS\033[m")' test: all - @for i in test/test*.js; do \\ - echo "default \$\$i: "; \\ + @for i in test/test*.js; do \\ + echo "default \$\$i: "; \\ build/default/node \$\$i && \$(PASS) || \$(FAIL); \\ - echo "debug \$\$i: "; \\ - build/debug/node \$\$i && \$(PASS) || \$(FAIL); \\ + if [ -e build/debug/node ]; then \\ + echo "debug \$\$i: "; \\ + build/debug/node \$\$i && \$(PASS) || \$(FAIL); \\ + fi; \\ done clean: From 194eeac0d9e607080d4f9bcf74a9470015931eef Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 16 Jun 2009 19:56:00 +0200 Subject: [PATCH 6/7] Add failing test for HTTP Client Reported by Hagen: http://groups.google.com/group/nodejs/browse_thread/thread/335b565360437b36 --- test/test-http-client-race.js | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 test/test-http-client-race.js diff --git a/test/test-http-client-race.js b/test/test-http-client-race.js new file mode 100644 index 0000000000..32b95d614d --- /dev/null +++ b/test/test-http-client-race.js @@ -0,0 +1,35 @@ +include("mjsunit.js"); +PORT = 8888; + +var server = new node.http.Server(function (req, res) { + res.sendHeader(200, [["content-type", "text/plain"]]); + res.sendBody("hello world\n"); + res.finish(); +}) +server.listen(PORT); + +var client = new node.http.Client(PORT); + +var body1 = ""; +var body2 = ""; + +client.get("/").finish(function (res1) { + res1.setBodyEncoding("utf8"); + + res1.onBody = function (chunk) { body1 += chunk; }; + + res1.onBodyComplete = function () { + client.get("/").finish(function (res2) { + res2.setBodyEncoding("utf8"); + res2.onBody = function (chunk) { body2 += chunk; }; + res2.onBodyComplete = function () { + server.close(); + }; + }); + }; +}); + +function onExit () { + assertEqual("hello world\n", body1); + assertEqual("hello world\n", body2); +} From d77f75774516fc9dfefe6c6523a94960df5100b5 Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 16 Jun 2009 20:53:15 +0200 Subject: [PATCH 7/7] Fix test-http-client-race bug --- src/http.js | 9 +++++++-- test/test-http-client-race.js | 13 ++++++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/http.js b/src/http.js index 6d5b2eda2f..ccbb994fe4 100644 --- a/src/http.js +++ b/src/http.js @@ -385,7 +385,12 @@ node.http.Client = function (port, host) { connection.connect(port, host); return; } - while (this === requests[0] && output.length > 0) { + //node.debug("HTTP CLIENT flush. readyState = " + connection.readyState); + while ( this === requests[0] + && output.length > 0 + && connection.readyState == "open" + ) + { var out = output.shift(); connection.send(out[0], out[1]); } @@ -403,7 +408,7 @@ node.http.Client = function (port, host) { connection.onConnect = function () { //node.debug("HTTP CLIENT onConnect. readyState = " + connection.readyState); - //node.debug("requests[0].uri = " + requests[0].uri); + //node.debug("requests[0].uri = '" + requests[0].uri + "'"); requests[0].flush(); }; diff --git a/test/test-http-client-race.js b/test/test-http-client-race.js index 32b95d614d..7761d4960d 100644 --- a/test/test-http-client-race.js +++ b/test/test-http-client-race.js @@ -3,7 +3,10 @@ PORT = 8888; var server = new node.http.Server(function (req, res) { res.sendHeader(200, [["content-type", "text/plain"]]); - res.sendBody("hello world\n"); + if (req.uri.path == "/1") + res.sendBody("hello world 1\n"); + else + res.sendBody("hello world 2\n"); res.finish(); }) server.listen(PORT); @@ -13,13 +16,13 @@ var client = new node.http.Client(PORT); var body1 = ""; var body2 = ""; -client.get("/").finish(function (res1) { +client.get("/1").finish(function (res1) { res1.setBodyEncoding("utf8"); res1.onBody = function (chunk) { body1 += chunk; }; res1.onBodyComplete = function () { - client.get("/").finish(function (res2) { + client.get("/2").finish(function (res2) { res2.setBodyEncoding("utf8"); res2.onBody = function (chunk) { body2 += chunk; }; res2.onBodyComplete = function () { @@ -30,6 +33,6 @@ client.get("/").finish(function (res1) { }); function onExit () { - assertEqual("hello world\n", body1); - assertEqual("hello world\n", body2); + assertEquals("hello world 1\n", body1); + assertEquals("hello world 2\n", body2); }