From 5c2389fada7e1e926d998f4c41ed5eeb82beb645 Mon Sep 17 00:00:00 2001 From: Ryan Date: Mon, 25 May 2009 13:17:35 +0200 Subject: [PATCH] Remove error codes from file on_completion callbacks. Use file.onError. The error codes still remain for the two general file system operations: rename and stat. Additionally I've removed the actionQueue for file system operations. They are sent directly into the thread pool. --- src/file.cc | 165 +++++++++++++++++++++-------------------- src/file.js | 105 +++++++++++++++----------- test/test-file-open.js | 7 +- website/node.html | 126 +++++++++++++++++++++---------- 4 files changed, 238 insertions(+), 165 deletions(-) diff --git a/src/file.cc b/src/file.cc index c014a70a9e..c8f6fae6ae 100644 --- a/src/file.cc +++ b/src/file.cc @@ -12,55 +12,40 @@ using namespace v8; using namespace node; -#define FD_SYMBOL v8::String::NewSymbol("fd") -#define ACTION_QUEUE_SYMBOL v8::String::NewSymbol("_actionQueue") -#define ENCODING_SYMBOL v8::String::NewSymbol("encoding") +#define FD_SYMBOL String::NewSymbol("fd") +#define ACTION_QUEUE_SYMBOL String::NewSymbol("_actionQueue") +#define ENCODING_SYMBOL String::NewSymbol("encoding") +#define CALLBACK_SYMBOL String::NewSymbol("callbaccallback") +#define POLL_ACTIONS_SYMBOL String::NewSymbol("_pollActions") -#define UTF8_SYMBOL v8::String::NewSymbol("utf8") -#define RAW_SYMBOL v8::String::NewSymbol("raw") - -static void -InitActionQueue (Handle handle) -{ - handle->Set(ACTION_QUEUE_SYMBOL, Array::New()); -} - -// This is the file system object which contains methods -// for accessing the file system (like rename, mkdir, etC). -// In javascript it is called "File". -static Persistent fs; +#define UTF8_SYMBOL String::NewSymbol("utf8") +#define RAW_SYMBOL String::NewSymbol("raw") void File::Initialize (Handle target) { - if (!fs.IsEmpty()) - return; - HandleScope scope; - fs = Persistent::New(target); - InitActionQueue(fs); - - // file system methods - NODE_SET_METHOD(fs, "_ffi_rename", FileSystem::Rename); - NODE_SET_METHOD(fs, "_ffi_stat", FileSystem::Stat); - NODE_SET_METHOD(fs, "strerror", FileSystem::StrError); + NODE_SET_METHOD(target, "rename", FileSystem::Rename); + NODE_SET_METHOD(target, "stat", FileSystem::Stat); + NODE_SET_METHOD(target, "strerror", FileSystem::StrError); - fs->Set(String::NewSymbol("STDIN_FILENO"), Integer::New(STDIN_FILENO)); - fs->Set(String::NewSymbol("STDOUT_FILENO"), Integer::New(STDOUT_FILENO)); - fs->Set(String::NewSymbol("STDERR_FILENO"), Integer::New(STDERR_FILENO)); + target->Set(String::NewSymbol("STDIN_FILENO"), Integer::New(STDIN_FILENO)); + target->Set(String::NewSymbol("STDOUT_FILENO"), Integer::New(STDOUT_FILENO)); + target->Set(String::NewSymbol("STDERR_FILENO"), Integer::New(STDERR_FILENO)); Local file_template = FunctionTemplate::New(File::New); file_template->InstanceTemplate()->SetInternalFieldCount(1); + // file methods NODE_SET_PROTOTYPE_METHOD(file_template, "_ffi_open", File::Open); NODE_SET_PROTOTYPE_METHOD(file_template, "_ffi_close", File::Close); NODE_SET_PROTOTYPE_METHOD(file_template, "_ffi_write", File::Write); NODE_SET_PROTOTYPE_METHOD(file_template, "_ffi_read", File::Read); file_template->InstanceTemplate()->SetAccessor(ENCODING_SYMBOL, File::GetEncoding, File::SetEncoding); - fs->Set(String::NewSymbol("File"), file_template->GetFunction()); + target->Set(String::NewSymbol("File"), file_template->GetFunction()); } Handle @@ -95,32 +80,15 @@ CallTopCallback (Handle handle, const int argc, Handle argv[]) { HandleScope scope; - Local queue_value = handle->Get(ACTION_QUEUE_SYMBOL); - assert(queue_value->IsArray()); - - Local queue = Local::Cast(queue_value); - Local top_value = queue->Get(Integer::New(0)); - if (top_value->IsObject()) { - Local top = top_value->ToObject(); - Local callback_value = top->Get(String::NewSymbol("callback")); - if (callback_value->IsFunction()) { - Handle callback = Handle::Cast(callback_value); - - TryCatch try_catch; - callback->Call(handle, argc, argv); - if(try_catch.HasCaught()) { - node::fatal_exception(try_catch); - return; - } - } - } - // poll_actions - Local poll_actions_value = handle->Get(String::NewSymbol("_pollActions")); + Local poll_actions_value = handle->Get(POLL_ACTIONS_SYMBOL); assert(poll_actions_value->IsFunction()); Handle poll_actions = Handle::Cast(poll_actions_value); - poll_actions->Call(handle, 0, NULL); + TryCatch try_catch; + poll_actions->Call(handle, argc, argv); + if(try_catch.HasCaught()) + node::fatal_exception(try_catch); } Handle @@ -134,6 +102,13 @@ FileSystem::Rename (const Arguments& args) String::Utf8Value path(args[0]->ToString()); String::Utf8Value new_path(args[1]->ToString()); + Persistent *callback = NULL; + if (args[2]->IsFunction()) { + Local l = Local::Cast(args[2]); + callback = new Persistent(); + *callback = Persistent::New(l); + } + node::eio_warmup(); eio_rename(*path, *new_path, EIO_PRI_DEFAULT, AfterRename, NULL); @@ -147,7 +122,18 @@ FileSystem::AfterRename (eio_req *req) const int argc = 1; Local argv[argc]; argv[0] = Integer::New(req->errorno); - CallTopCallback(fs, argc, argv); + + if (req->data) { + Persistent *callback = reinterpret_cast*>(req->data); + + TryCatch try_catch; + (*callback)->Call(Context::GetCurrent()->Global(), argc, argv); + if(try_catch.HasCaught()) + node::fatal_exception(try_catch); + + free(callback); + } + return 0; } @@ -161,8 +147,15 @@ FileSystem::Stat (const Arguments& args) String::Utf8Value path(args[0]->ToString()); + Persistent *callback = NULL; + if (args[1]->IsFunction()) { + Local l = Local::Cast(args[1]); + callback = new Persistent(); + *callback = Persistent::New(l); + } + node::eio_warmup(); - eio_stat(*path, EIO_PRI_DEFAULT, AfterStat, NULL); + eio_stat(*path, EIO_PRI_DEFAULT, AfterStat, callback); return Undefined(); } @@ -180,7 +173,7 @@ FileSystem::AfterStat (eio_req *req) argv[1] = stats; if (req->result == 0) { - struct stat *s = static_cast(req->ptr2); + struct stat *s = reinterpret_cast(req->ptr2); /* ID of device containing file */ stats->Set(NODE_SYMBOL("dev"), Integer::New(s->st_dev)); @@ -210,8 +203,17 @@ FileSystem::AfterStat (eio_req *req) stats->Set(NODE_SYMBOL("ctime"), Date::New(1000*static_cast(s->st_ctime))); } - CallTopCallback(fs, argc, argv); - + if (req->data) { + Persistent *callback = reinterpret_cast*>(req->data); + + TryCatch try_catch; + (*callback)->Call(Context::GetCurrent()->Global(), argc, argv); + if(try_catch.HasCaught()) + node::fatal_exception(try_catch); + + free(callback); + } + return 0; } @@ -237,7 +239,7 @@ File::File (Handle handle) { HandleScope scope; encoding_ = RAW; - InitActionQueue(handle); + handle->Set(ACTION_QUEUE_SYMBOL, Array::New()); } File::~File () @@ -277,7 +279,7 @@ File::Close (const Arguments& args) int File::AfterClose (eio_req *req) { - File *file = static_cast(req->data); + File *file = reinterpret_cast(req->data); if (req->result == 0) { file->handle_->Delete(FD_SYMBOL); @@ -338,7 +340,7 @@ File::Open (const Arguments& args) int File::AfterOpen (eio_req *req) { - File *file = static_cast(req->data); + File *file = reinterpret_cast(req->data); HandleScope scope; if(req->result >= 0) { @@ -364,6 +366,8 @@ File::Write (const Arguments& args) File *file = NODE_UNWRAP(File, args.Holder()); + off_t pos = args[1]->IntegerValue(); + char *buf = NULL; size_t length = 0; @@ -371,14 +375,14 @@ File::Write (const Arguments& args) // utf8 encoding Local string = args[0]->ToString(); length = string->Utf8Length(); - buf = static_cast(malloc(length)); + buf = reinterpret_cast(malloc(length)); string->WriteUtf8(buf, length); } else if (args[0]->IsArray()) { // raw encoding Local array = Local::Cast(args[0]); length = array->Length(); - buf = static_cast(malloc(length)); + buf = reinterpret_cast(malloc(length)); for (unsigned int i = 0; i < length; i++) { Local int_value = array->Get(Integer::New(i)); buf[i] = int_value->Int32Value(); @@ -389,8 +393,6 @@ File::Write (const Arguments& args) return Undefined(); } - off_t pos = args[1]->IntegerValue(); - if (file->handle_->Has(FD_SYMBOL) == false) { printf("trying to write to a bad fd!\n"); return Undefined(); @@ -408,9 +410,9 @@ File::Write (const Arguments& args) int File::AfterWrite (eio_req *req) { - File *file = static_cast(req->data); + File *file = reinterpret_cast(req->data); - //char *buf = static_cast(req->ptr2); + //char *buf = reinterpret_cast(req->ptr2); free(req->ptr2); ssize_t written = req->result; @@ -429,20 +431,22 @@ File::AfterWrite (eio_req *req) Handle File::Read (const Arguments& args) { - if (args.Length() < 1) return Undefined(); - if (!args[0]->IsNumber()) return Undefined(); - if (!args[1]->IsNumber()) return Undefined(); - HandleScope scope; + + if (args.Length() < 1 || !args[0]->IsNumber() || !args[1]->IsNumber()) { + return ThrowException(String::New("Bad parameter for _ffi_read()")); + } + File *file = NODE_UNWRAP(File, args.Holder()); - size_t length = args[0]->IntegerValue(); - off_t pos = args[1]->IntegerValue(); + size_t len = args[0]->IntegerValue(); + off_t pos = args[1]->IntegerValue(); int fd = file->GetFD(); + assert(fd >= 0); - // NOTE: NULL pointer tells eio to allocate it itself + // NOTE: 2nd param: NULL pointer tells eio to allocate it itself node::eio_warmup(); - eio_read(fd, NULL, length, pos, EIO_PRI_DEFAULT, File::AfterRead, file); + eio_read(fd, NULL, len, pos, EIO_PRI_DEFAULT, File::AfterRead, file); file->Attach(); return Undefined(); @@ -451,32 +455,33 @@ File::Read (const Arguments& args) int File::AfterRead (eio_req *req) { - File *file = static_cast(req->data); + File *file = reinterpret_cast(req->data); HandleScope scope; const int argc = 2; Local argv[argc]; argv[0] = Integer::New(req->errorno); - char *buf = static_cast(req->ptr2); + char *buf = reinterpret_cast(req->ptr2); - if(req->result == 0) { + if (req->result == 0) { // eof argv[1] = Local::New(Null()); } else { - size_t length = req->result; + size_t len = req->result; if (file->encoding_ == UTF8) { // utf8 encoding argv[1] = String::New(buf, req->result); } else { // raw encoding - Local array = Array::New(length); - for (unsigned int i = 0; i < length; i++) { + Local array = Array::New(len); + for (unsigned int i = 0; i < len; i++) { array->Set(Integer::New(i), Integer::New(buf[i])); } argv[1] = array; } } + CallTopCallback(file->handle_, argc, argv); file->Detach(); diff --git a/src/file.js b/src/file.js index e645863a46..a4caafc856 100644 --- a/src/file.js +++ b/src/file.js @@ -1,13 +1,5 @@ -node.fs.rename = function (file1, file2, callback) { - this._addAction("rename", [file1, file2], callback); -}; - -node.fs.stat = function (path, callback) { - this._addAction("stat", [path], callback); -}; - node.fs.exists = function (path, callback) { - this._addAction("stat", [path], function (status) { + node.fs.stat(path, function (status) { callback(status == 0); }); } @@ -16,16 +8,17 @@ node.fs.cat = function (path, encoding, callback) { var file = new node.fs.File(); file.encoding = encoding; - var content = ""; - if (file.encoding == "raw") content = []; + file.onError = function (method, errno, msg) { + callback(-1); + }; + var content = (file.encoding == "raw" ? "" : []); var pos = 0; var chunkSize = 10*1024; function readChunk () { - file.read(chunkSize, pos, function (status, chunk) { + file.read(chunkSize, pos, function (chunk) { if (chunk) { - if (chunk.constructor == String) content += chunk; else @@ -40,22 +33,11 @@ node.fs.cat = function (path, encoding, callback) { }); } - file.open(path, "r", function (status) { - if (status == 0) - readChunk(); - else - callback (status); + file.open(path, "r", function () { + readChunk(); }); } -node.fs.File.prototype.puts = function (data, callback) { - this.write(data + "\n", -1, callback); -}; - -node.fs.File.prototype.print = function (data, callback) { - this.write(data, -1, callback); -}; - node.fs.File.prototype.open = function (path, mode, callback) { this._addAction("open", [path, mode], callback); }; @@ -64,12 +46,20 @@ node.fs.File.prototype.close = function (callback) { this._addAction("close", [], callback); }; +node.fs.File.prototype.read = function (length, pos, callback) { + this._addAction("read", [length, pos], callback); +}; + node.fs.File.prototype.write = function (buf, pos, callback) { this._addAction("write", [buf, pos], callback); }; -node.fs.File.prototype.read = function (length, pos, callback) { - this._addAction("read", [length, pos], callback); +node.fs.File.prototype.print = function (data, callback) { + this.write(data, -1, callback); +}; + +node.fs.File.prototype.puts = function (data, callback) { + this.write(data + "\n", -1, callback); }; // Some explanation of the File binding. @@ -84,38 +74,65 @@ node.fs.File.prototype.read = function (length, pos, callback) { // the member _actionQueue = [] // // Any of the methods called on a file are put into this queue. When they -// reach the head of the queue they will be executed. C++ calles the +// reach the head of the queue they will be executed. C++ calls the // method _pollActions each time it becomes idle. If there is no action -// currently being executed then _pollActions will not be called. Thus when -// actions are added to an empty _actionQueue, they should be immediately +// currently being executed then _pollActions will not be called. When +// actions are added to an empty _actionQueue, they will be immediately // executed. // -// When an action has completed, the C++ side is going to look at the first +// When an action has completed, the C++ side is looks at the first // element of _actionQueue in order to get a handle on the callback // function. Only after that completion callback has been made can the // action be shifted out of the queue. // -// See File::CallTopCallback() in file.cc to see the other side of the -// binding. -node.fs._addAction = node.fs.File.prototype._addAction = function (method, args, callback) { - this._actionQueue.push({ method: method - , callback: callback - , args: args - }); +// See File::CallTopCallback() in file.cc for the other side of the binding. + + +node.fs.File.prototype._addAction = function (method, args, callback) { + // This adds a method to the queue. + var action = { method: method + , callback: callback + , args: args + }; + this._actionQueue.push(action); + + // If the queue was empty, immediately call the method. if (this._actionQueue.length == 1) this._act(); -} +}; -node.fs._act = node.fs.File.prototype._act = function () { +node.fs.File.prototype._act = function () { + // peek at the head of the queue var action = this._actionQueue[0]; - if (action) - // TODO FIXME what if the action throws an error? + if (action) { + // execute the c++ version of the method. the c++ version + // is gotten by appending "_ffi_" to the method name. this["_ffi_" + action.method].apply(this, action.args); + } }; // called from C++ after each action finishes // (i.e. when it returns from the thread pool) -node.fs._pollActions = node.fs.File.prototype._pollActions = function () { +node.fs.File.prototype._pollActions = function () { + var action = this._actionQueue[0]; + + var errno = arguments[0]; + + if (errno < 0) { + if (this.onError) + this.onError(action.method, errno, node.fs.strerror(errno)); + this._actionQueue = []; // empty the queue. + return; + } + + var rest = []; + for (var i = 1; i < arguments.length; i++) + rest.push(arguments[i]); + + if (action.callback) + action.callback.apply(this, rest); + this._actionQueue.shift(); + this._act(); }; diff --git a/test/test-file-open.js b/test/test-file-open.js index 2e3f7ddbbf..98da2f7dff 100644 --- a/test/test-file-open.js +++ b/test/test-file-open.js @@ -7,8 +7,11 @@ function onLoad () { var x = node.path.join(fixtures, "x.txt"); file = new node.fs.File; - file.open(x, "r", function (status) { - assertTrue(status == 0); + file.onError = function (method, errno, msg) { + assertTrue(false); + }; + + file.open(x, "r", function () { assert_count += 1; file.close(); }); diff --git a/website/node.html b/website/node.html index 462614659c..213d8d1c41 100644 --- a/website/node.html +++ b/website/node.html @@ -80,11 +80,11 @@ a:hover { text-decoration: underline; }
  1. Benchmarks
  2. Download
    3. -
  3. Build
    4. +
  4. Build
  5. API
    1. Timers -
    2. File System I/O +
    3. File I/O
    4. TCP
      1. Server @@ -166,12 +166,15 @@ always have a capital first letter.
        puts(string, callback)
        -
        Outputs the string and a trailing new-line to stdout. +
        + Alias for stdout.puts(). + + Outputs the string and a trailing new-line to stdout.

        The callback argument is optional and mostly useless: it will notify the user when the operation has completed. Everything in node is asynchronous; puts() is no exception. This might seem ridiculous - but, if for example, one is piping their output into an NFS'd file, - printf() will block. + but, if for example, one is piping stdout into an NFS file, + printf() will block from network latency. There is an internal queue for puts() output, so you can be assured that output will be displayed in the order it was called.

        @@ -204,45 +207,80 @@ always have a capital first letter.
        Stops a interval from triggering.
        -

        node.File

        +

        node.fs

        -

        File system I/O has always been tricky because there are not any portable -non-blocking ways to do it. To get around this, Node uses Because there are not non-blocking ways to do it, asynchronous file I/O is +tricky. Node handles file I/O by employing an internal thread pool -to execute file system calls asynchronously. - - -

        All file I/O calls are rather thin wrappers around standard POSIX functions. -All calls have an optional last callback parameter +to execute file system calls. +

        Internal request queues exist for each file object so that multiple commands +can be issued at once without worry that they will be executed out-of-order. +Thus the following is safe: - - -

        Internal request queues exist for each file object so multiple commands can -be issued at once without worry that they will reach the file system out of -order. Thus the following is safe: 

        -var file = new node.File();
+var file = new node.fs.File();
 file.open("/tmp/blah", "w+");
-file.write("hello ");
+file.write("hello");
 file.write("world");
 file.close();
        -Additionally there is a process-wide queue for all commands which operate on -the file system directory structure (like rename and -unlink). It's important to understand that all of these request queues are -distinct. If, for example, you do + +

        +It's important to understand that the request queues are local to a single file. +If one does 

        fileA.write("hello");
 fileB.write("world");
        -it could be that -first fileB gets written to and then fileA gets written to. -So if a certain operation order is needed involving multiple files, use the +it could be that fileB gets written to before fileA +is written to. +If a certain operation order is needed involving multiple files, use the completion callbacks: 
        fileA.write("hello", function () {
   fileB.write("world");
 });
        -
        node.File.rename()
        +
        new node.fs.File
        +
        Creates a new file object.
        + +
        file.open(path, mode, on_completion)
        +
        Opens the file at path. +

        mode is a string: + "r" open for reading and writing. + "r+" open for only reading. + "w" create a new file for reading and writing; if it + already exists truncate it. + "w+" create a new file for writing only; if it already + exists truncate it. + "a" create a new file for writing and reading. Writes + append to the end of the file. + "a+" +

        The on_completion is a callback that is made without + arguments when the operation completes. It is optional + If an error occurred the on_completion callback will not be + called, but the file.onError will be called. +

        + +
        file.read(length, position, on_completion)
        +
        +
        + +
        file.write(data, position, on_completion)
        +
        +
        + +
        file.close(on_completion)
        +
        +
        +
        + +

        File System Operations

        + +
        +
        node.fs.rename(path1, path2, on_completion)
        +
        +
        + +
        node.fs.stat(path1, on_completion)
        @@ -250,6 +288,10 @@ completion callbacks:

        node.tcp

        +

        node.tcp.Server

        + +

        node.tcp.Connection

        +

        node.http

        Node provides a web server and client interface. The interface is rather @@ -410,10 +452,11 @@ res.sendHeader(200, [ ["Content-Length", body.length]

        node.http.Client

        -

        An HTTP client is constructed with a server address as its argument, then -the user issues one or more requests. Depending on the server connected to, -the client might pipeline the requests or reestablish the connection after each -connection. Currently the client does not pipeline requests. +

        An HTTP client is constructed with a server address as its argument, the +returned handle is then used to issue one or more requests. Depending on the +server connected to, the client might pipeline the requests or reestablish the +connection after each connection. +Currently the implementation does not pipeline requests.

        Example of connecting to google.com 

        @@ -441,7 +484,9 @@ request is issued.
   
        client.post(path, request_headers);
        
   
        client.del(path, request_headers);
        
   
        client.put(path, request_headers);
        
-  
         Issues a request. 
+  
         Issues a request; if necessary establishes connection. 
+
+  
        
     request_headers is optional. 
     request_headers should be an array of 2-element arrays. 
     Additional request headers might be added internally by Node.
@@ -450,15 +495,18 @@ request is issued.
     
        Important: the request is not complete. This method only sends the
 header of the request. One needs to call req.finish() to finalize
 the request and retrieve the response.  (This sounds convoluted but it provides
-a chance for the user to stream a body to the server with
-req.sendBody.  GET and HEAD requests
-normally are without bodies but HTTP does not forbid it, so neither do we.)
+a chance for the user to stream a body to the server with req.sendBody().) 
+
+    
         GET and
+HEAD requests normally are without bodies but HTTP does not forbid
+it, so neither do we.
 
 
 
 
        node.http.ClientRequest
        
 
-
        This object created internally and returned from the request methods of a
+
        This object is created internally and returned from the request methods of a
 node.http.Client. It represents an in-progress request
 whose header has already been sent. 
 
@@ -500,7 +548,7 @@ read.
 
 
        
   
        res.statusCode
        
-  
        The 3-digit HTTP response status code. (E.G. 404.)
        
+  
        The 3-digit HTTP response status code. E.G. 404.
        
 
   
        res.httpVersion
        
   
        The HTTP version of the connected-to server. Probably either 
@@ -513,7 +561,7 @@ read.
 
   
        res.onBody
        
   
        Callback. Should be set by the user to be informed of when a piece
-  of the message body is received.
+  of the response body is received.
   A chunk of the body is given as the single argument. The transfer-encoding
   has been removed.