Skip to content

Commit 741e3fa

Browse files
ryry
committed
HTTP works somewhat on net2 now
However it's not working very well: Hitting a 'hello world' server with many requests (ab -t 60 -c 10) will cause it to crash with the following error. Obtained 3 stack frames. ./node(_Z11print_tracev+0x1c) [0x80d1b3c] ./node(_ZN4node6Parser7ExecuteERKN2v89ArgumentsE+0x69) [0x80d3759] ./node [0x811f44b] TypeError: Already parsing a buffer at Socket.<anonymous> (/home/ryan/projects/node/lib/http2.js:393:20) at IOWatcher.callback (/home/ryan/projects/node/lib/net.js:81:12) at node.js:985:9 at node.js:989:1
1 parent dda1d68 commit 741e3fa

7 files changed

Lines changed: 124 additions & 31 deletions

File tree

‎lib/net.js‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ var getaddrinfo = process.getaddrinfo;
2828
var needsLookup = process.needsLookup;
2929
var EINPROGRESS = process.EINPROGRESS;
3030
var ENOENT = process.ENOENT;
31-
var END_OF_FILE = 42;
31+
var END_OF_FILE = 0;
3232

3333
function Socket (peerInfo) {
3434
process.EventEmitter.call();
@@ -39,6 +39,7 @@ function Socket (peerInfo) {
3939
self.recvBuffer = null;
4040

4141
self.readWatcher = new IOWatcher();
42+
self.readWatcher.host = this;
4243
self.readWatcher.callback = function () {
4344
// If this is the first recv (recvBuffer doesn't exist) or we've used up
4445
// most of the recvBuffer, allocate a new one.
@@ -61,9 +62,9 @@ function Socket (peerInfo) {
6162
}
6263
} else {
6364
bytesRead = read(self.fd,
64-
self.recvBuffer,
65-
self.recvBuffer.used,
66-
self.recvBuffer.length - self.recvBuffer.used);
65+
self.recvBuffer,
66+
self.recvBuffer.used,
67+
self.recvBuffer.length - self.recvBuffer.used);
6768
}
6869

6970
debug('bytesRead ' + bytesRead + '\n');
@@ -99,6 +100,7 @@ function Socket (peerInfo) {
99100
}
100101
};
101102
self.writeWatcher = new IOWatcher();
103+
self.writeWatcher.host = this;
102104
self.writeWatcher.callback = self._doFlush;
103105
self.writable = false;
104106

@@ -131,6 +133,7 @@ Socket.prototype._allocateNewRecvBuf = function () {
131133

132134
var newBufferSize = 1024; // TODO make this adjustable from user API
133135

136+
/*
134137
if (toRead) {
135138
// Is the extra system call even worth it?
136139
var bytesToRead = toRead(self.fd);
@@ -145,6 +148,7 @@ Socket.prototype._allocateNewRecvBuf = function () {
145148
newBufferSize = 128;
146149
}
147150
}
151+
*/
148152

149153
self.recvBuffer = new process.Buffer(newBufferSize);
150154
self.recvBuffer.used = 0;
@@ -421,6 +425,7 @@ Socket.prototype.forceClose = function (exception) {
421425

422426
this.writeWatcher.stop();
423427
this.readWatcher.stop();
428+
424429
close(this.fd);
425430
debug('close socket ' + this.fd);
426431
this.fd = null;
@@ -455,6 +460,7 @@ function Server (listener) {
455460
}
456461

457462
self.watcher = new IOWatcher();
463+
self.watcher.host = self;
458464
self.watcher.callback = function (readable, writeable) {
459465
while (self.fd) {
460466
var peerInfo = accept(self.fd);

‎src/node_buffer.cc‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,24 +33,15 @@ bool IsBuffer(v8::Handle<v8::Value> val) {
3333

3434

3535
/* Determines the absolute position for a relative offset */
36-
static inline size_t buffer_abs_off(buffer *buffer, size_t off) {
36+
size_t buffer_abs_off(buffer *buffer, size_t off) {
3737
struct buffer *root = buffer_root(buffer);
3838
off += buffer->offset;
3939
return MIN(root->length, off);
4040
}
4141

4242

43-
static inline void buffer_ref(struct buffer *buffer) {
44-
assert(buffer->root == NULL);
45-
buffer->refs++;
46-
}
47-
48-
49-
static inline void buffer_unref(struct buffer *buffer) {
50-
assert(buffer->root == NULL);
51-
assert(buffer->refs > 0);
52-
buffer->refs--;
53-
if (buffer->refs == 0 && buffer->weak) free(buffer);
43+
void buffer_ref(struct buffer *buffer) {
44+
buffer_root(buffer)->refs++;
5445
}
5546

5647

@@ -69,15 +60,26 @@ static void RootWeakCallback(Persistent<Value> value, void *data)
6960
struct buffer *buffer = static_cast<struct buffer*>(data);
7061
assert(buffer->root == NULL); // this is the root
7162
assert(value == buffer->handle);
72-
buffer->handle.Dispose();
63+
value.ClearWeak();
7364
if (buffer->refs) {
7465
buffer->weak = true;
7566
} else {
67+
buffer->handle.Dispose();
7668
free(buffer);
7769
}
7870
}
7971

8072

73+
void buffer_unref(struct buffer *buffer) {
74+
struct buffer * root = buffer_root(buffer);
75+
assert(root->refs > 0);
76+
root->refs--;
77+
if (root->refs == 0 && root->weak) {
78+
root->handle.MakeWeak(root, RootWeakCallback);
79+
}
80+
}
81+
82+
8183
static void SliceWeakCallback(Persistent<Value> value, void *data)
8284
{
8385
struct buffer *buffer = static_cast<struct buffer*>(data);

‎src/node_buffer.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ static inline size_t buffer_remaining(struct buffer *buffer, size_t off) {
5656
return end - buffer_p(buffer, off);
5757
}
5858

59-
}
59+
void buffer_ref(struct buffer *buffer);
60+
void buffer_unref(struct buffer *buffer);
61+
62+
} // namespace node buffer
6063

6164
#endif // NODE_BUFFER

‎src/node_http_parser.cc‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,30 @@
1919
// No copying is performed when slicing the buffer, only small reference
2020
// allocations.
2121

22+
#include <execinfo.h>
23+
#include <stdio.h>
24+
#include <stdlib.h>
25+
26+
/* Obtain a backtrace and print it to stdout. */
27+
void
28+
print_trace (void)
29+
{
30+
void *array[10];
31+
size_t size;
32+
char **strings;
33+
size_t i;
34+
35+
size = backtrace (array, 10);
36+
strings = backtrace_symbols (array, size);
37+
38+
printf ("Obtained %zd stack frames.\n", size);
39+
40+
for (i = 0; i < size; i++)
41+
printf ("%s\n", strings[i]);
42+
43+
free (strings);
44+
}
45+
2246
namespace node {
2347

2448
using namespace v8;
@@ -93,6 +117,7 @@ static Persistent<String> should_keep_alive_sym;
93117
, Integer::New(length) \
94118
}; \
95119
Local<Value> ret = cb->Call(parser->handle_, 3, argv); \
120+
assert(parser->buffer_); \
96121
return ret.IsEmpty() ? -1 : 0; \
97122
}
98123

@@ -141,6 +166,10 @@ class Parser : public ObjectWrap {
141166
parser_.data = this;
142167
}
143168

169+
~Parser() {
170+
assert(buffer_ == NULL && "Destroying a parser while it's parsing");
171+
}
172+
144173
DEFINE_HTTP_CB(on_message_begin)
145174
DEFINE_HTTP_CB(on_message_complete)
146175

@@ -215,6 +244,7 @@ class Parser : public ObjectWrap {
215244
Parser *parser = ObjectWrap::Unwrap<Parser>(args.This());
216245

217246
if (parser->buffer_) {
247+
print_trace();
218248
return ThrowException(Exception::TypeError(
219249
String::New("Already parsing a buffer")));
220250
}
@@ -243,9 +273,13 @@ class Parser : public ObjectWrap {
243273
// Assign 'buffer_' while we parse. The callbacks will access that varible.
244274
parser->buffer_ = buffer;
245275

276+
buffer_ref(parser->buffer_);
277+
246278
size_t nparsed =
247279
http_parser_execute(&(parser->parser_), buffer_p(buffer, off), len);
248280

281+
buffer_unref(parser->buffer_);
282+
249283
// Unassign the 'buffer_' variable
250284
assert(parser->buffer_);
251285
parser->buffer_ = NULL;
@@ -266,6 +300,17 @@ class Parser : public ObjectWrap {
266300
return scope.Close(nparsed_obj);
267301
}
268302

303+
static Handle<Value> ExecuteEOF(const Arguments& args) {
304+
HandleScope scope;
305+
306+
Parser *parser = ObjectWrap::Unwrap<Parser>(args.This());
307+
308+
assert(!parser->buffer_);
309+
http_parser_execute(&parser->parser_, NULL, 0);
310+
311+
return Undefined();
312+
}
313+
269314

270315
private:
271316

@@ -282,6 +327,7 @@ void InitHttpParser(Handle<Object> target) {
282327
//t->SetClassName(String::NewSymbol("HTTPParser"));
283328

284329
NODE_SET_PROTOTYPE_METHOD(t, "execute", Parser::Execute);
330+
NODE_SET_PROTOTYPE_METHOD(t, "executeEOF", Parser::ExecuteEOF);
285331

286332
target->Set(String::NewSymbol("HTTPParser"), t->GetFunction());
287333

‎src/node_io_watcher.cc‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,16 +59,17 @@ void IOWatcher::Callback(EV_P_ ev_io *w, int revents) {
5959

6060

6161
//
62-
// var io = new process.IOWatcher(function (readable, writable) {
63-
//
64-
// });
62+
// var io = new process.IOWatcher();
63+
// io.callback = function (readable, writable) { ... };
6564
// io.set(fd, true, false);
6665
// io.start();
6766
//
6867
Handle<Value> IOWatcher::New(const Arguments& args) {
6968
HandleScope scope;
7069

7170
IOWatcher *s = new IOWatcher();
71+
72+
7273
s->Wrap(args.This());
7374

7475
return args.This();
@@ -78,19 +79,18 @@ Handle<Value> IOWatcher::New(const Arguments& args) {
7879
Handle<Value> IOWatcher::Start(const Arguments& args) {
7980
HandleScope scope;
8081

81-
IOWatcher *io = ObjectWrap::Unwrap<IOWatcher>(args.Holder());
82+
IOWatcher *io = Unwrap(args.Holder());
8283

8384
ev_io_start(EV_DEFAULT_UC_ &io->watcher_);
84-
85-
io->Ref();
85+
assert(ev_is_active(&io->watcher_));
8686

8787
return Undefined();
8888
}
8989

9090
Handle<Value> IOWatcher::Set(const Arguments& args) {
9191
HandleScope scope;
9292

93-
IOWatcher *io = ObjectWrap::Unwrap<IOWatcher>(args.Holder());
93+
IOWatcher *io = Unwrap(args.Holder());
9494

9595
if (!args[0]->IsInt32()) {
9696
return ThrowException(Exception::TypeError(
@@ -122,16 +122,16 @@ Handle<Value> IOWatcher::Set(const Arguments& args) {
122122

123123
Handle<Value> IOWatcher::Stop(const Arguments& args) {
124124
HandleScope scope;
125-
IOWatcher *io = ObjectWrap::Unwrap<IOWatcher>(args.Holder());
125+
IOWatcher *io = Unwrap(args.This());
126126
io->Stop();
127127
return Undefined();
128128
}
129129

130130

131131
void IOWatcher::Stop () {
132-
if (watcher_.active) {
132+
if (ev_is_active(&watcher_)) {
133133
ev_io_stop(EV_DEFAULT_UC_ &watcher_);
134-
Unref();
134+
assert(!ev_is_active(&watcher_));
135135
}
136136
}
137137

‎src/node_io_watcher.h‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,33 +7,68 @@
77

88
namespace node {
99

10-
class IOWatcher : ObjectWrap {
10+
class IOWatcher {
1111
public:
1212
static void Initialize(v8::Handle<v8::Object> target);
1313

1414
protected:
1515
static v8::Persistent<v8::FunctionTemplate> constructor_template;
1616

17-
IOWatcher() : ObjectWrap() {
17+
IOWatcher() {
1818
ev_init(&watcher_, IOWatcher::Callback);
1919
watcher_.data = this;
2020
}
2121

2222
~IOWatcher() {
23-
ev_io_stop(EV_DEFAULT_UC_ &watcher_);
23+
assert(!ev_is_active(&watcher_));
2424
}
2525

2626
static v8::Handle<v8::Value> New(const v8::Arguments& args);
2727
static v8::Handle<v8::Value> Start(const v8::Arguments& args);
2828
static v8::Handle<v8::Value> Stop(const v8::Arguments& args);
2929
static v8::Handle<v8::Value> Set(const v8::Arguments& args);
3030

31+
inline void Wrap(v8::Handle<v8::Object> handle) {
32+
assert(handle_.IsEmpty());
33+
assert(handle->InternalFieldCount() > 0);
34+
handle_ = v8::Persistent<v8::Object>::New(handle);
35+
handle_->SetInternalField(0, v8::External::New(this));
36+
MakeWeak();
37+
}
38+
39+
inline void MakeWeak(void) {
40+
handle_.MakeWeak(this, WeakCallback);
41+
}
42+
43+
44+
3145
private:
3246
static void Callback(EV_P_ ev_io *watcher, int revents);
3347

48+
static void WeakCallback (v8::Persistent<v8::Value> value, void *data)
49+
{
50+
IOWatcher *io = static_cast<IOWatcher*>(data);
51+
assert(value == io->handle_);
52+
if (!ev_is_active(&io->watcher_)) {
53+
value.Dispose();
54+
delete io;
55+
} else {
56+
//value.ClearWeak();
57+
io->MakeWeak();
58+
}
59+
}
60+
61+
static IOWatcher* Unwrap(v8::Handle<v8::Object> handle) {
62+
assert(!handle.IsEmpty());
63+
assert(handle->InternalFieldCount() > 0);
64+
return static_cast<IOWatcher*>(v8::Handle<v8::External>::Cast(
65+
handle->GetInternalField(0))->Value());
66+
}
67+
3468
void Stop();
3569

3670
ev_io watcher_;
71+
v8::Persistent<v8::Object> handle_;
3772
};
3873

3974
} // namespace node

‎src/node_object_wrap.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ class ObjectWrap {
5353
assert(!handle_.IsEmpty());
5454
assert(handle_.IsWeak());
5555
refs_++;
56+
MakeWeak();
5657
}
5758

5859
/* Unref() marks an object as detached from the event loop. This is its

0 commit comments

Comments
 (0)