From 595d6dca5503b5eb0354f5b6f0cc908d06c1d46d Mon Sep 17 00:00:00 2001 From: Ernest Wong Date: Wed, 19 Sep 2018 22:10:13 +1200 Subject: [PATCH] Filesystem: Refactor inodedata accesses to async methods The "// jshint ignore:line" comments are pretty messy, so squint your eyes. They're systematically placed, so we can regex it out when jshint's new version finally arrives. Using async/await instead of callbacks due to callback hell, and it also helps minimising the diff) --- lib/9p.js | 20 ++++---- lib/filesystem.js | 111 ++++++++++++++++++++++++++--------------- src/browser/starter.js | 36 ++++++------- 3 files changed, 97 insertions(+), 70 deletions(-) diff --git a/lib/9p.js b/lib/9p.js index fc7415ea..03dcd461 100644 --- a/lib/9p.js +++ b/lib/9p.js @@ -250,7 +250,7 @@ Virtio9p.prototype.SendReply = function (bufchain) { this.virtqueue.flush_replies(); }; -Virtio9p.prototype.ReceiveRequest = function (bufchain) { +Virtio9p.prototype.ReceiveRequest = async function (bufchain) { // jshint ignore:line // TODO: split into header + data blobs to avoid unnecessary copying. const buffer = new Uint8Array(bufchain.length_readable); bufchain.get_next_blob(buffer); @@ -552,7 +552,7 @@ Virtio9p.prototype.ReceiveRequest = function (bufchain) { inode.mtime = req[8]; } if (req[1] & P9_SETATTR_SIZE) { - this.fs.ChangeSize(this.fids[fid].inodeid, req[5]); + await this.fs.ChangeSize(this.fids[fid].inodeid, req[5]); // jshint ignore:line } this.BuildReply(id, tag, 0); this.SendReply(bufchain); @@ -593,7 +593,7 @@ Virtio9p.prototype.ReceiveRequest = function (bufchain) { this.fs.OpenInode(this.fids[fid].inodeid, undefined); this.fs.AddEvent(this.fids[fid].inodeid, - function() { + async function() { // jshint ignore:line this.bus.send("9p-read-end", [this.fids[fid].dbg_name, count]); const inodeid = this.fids[fid].inodeid; @@ -610,15 +610,15 @@ Virtio9p.prototype.ReceiveRequest = function (bufchain) { // See http://ericvh.github.io/9p-rfc/rfc9p2000.html#anchor30 count = 0; } - const data = this.fs.Read(inodeid, offset, count); + const data = await this.fs.Read(inodeid, offset, count); // jshint ignore:line if(data) { this.replybuffer.set(data, 7 + 4); } marshall.Marshall(["w"], [count], this.replybuffer, 7); this.BuildReply(id, tag, 4 + count); this.SendReply(bufchain); - }.bind(this) - ); + }.bind(this) // jshint ignore:line + ); // jshint ignore:line } break; @@ -640,7 +640,7 @@ Virtio9p.prototype.ReceiveRequest = function (bufchain) { else { // XXX: Size of the subarray is unchecked - this.fs.Write(this.fids[fid].inodeid, offset, count, buffer.subarray(state.offset)); + await this.fs.Write(this.fids[fid].inodeid, offset, count, buffer.subarray(state.offset)); // jshint ignore:line } this.bus.send("9p-write-end", [filename, count]); @@ -657,7 +657,7 @@ Virtio9p.prototype.ReceiveRequest = function (bufchain) { var newdirfid = req[2]; var newname = req[3]; message.Debug("[renameat]: oldname=" + oldname + " newname=" + newname); - var ret = this.fs.Rename(this.fids[olddirfid].inodeid, oldname, this.fids[newdirfid].inodeid, newname); + var ret = await this.fs.Rename(this.fids[olddirfid].inodeid, oldname, this.fids[newdirfid].inodeid, newname); // jshint ignore:line if (ret < 0) { let error_message = ""; if(ret === -ENOENT) error_message = "No such file or directory"; @@ -790,7 +790,7 @@ Virtio9p.prototype.ReceiveRequest = function (bufchain) { var req = marshall.Unmarshall(["w"], buffer, state); message.Debug("[clunk]: fid=" + req[0]); if (this.fids[req[0]] && this.fids[req[0]].inodeid >= 0) { - this.fs.CloseInode(this.fids[req[0]].inodeid); + await this.fs.CloseInode(this.fids[req[0]].inodeid); // jshint ignore:line this.fids[req[0]].inodeid = -1; this.fids[req[0]].type = FID_NONE; } @@ -844,4 +844,4 @@ Virtio9p.prototype.ReceiveRequest = function (bufchain) { //consistency checks if there are problems with the filesystem //this.fs.Check(); -}; +}; // jshint ignore:line diff --git a/lib/filesystem.js b/lib/filesystem.js index 14939f96..fdf2d091 100644 --- a/lib/filesystem.js +++ b/lib/filesystem.js @@ -303,15 +303,16 @@ FS.prototype.LoadFile = function(idx) { if(this.baseurl) { LoadBinaryResource(this.baseurl + inode.sha256sum, - function(buffer){ - var data = this.inodedata[idx] = new Uint8Array(buffer); + async function(buffer){ // jshint ignore:line + const data = new Uint8Array(buffer); + await this.set_data(idx, data); // jshint ignore:line inode.size = data.length; // correct size if the previous was wrong. inode.status = STATUS_OK; this.filesinloadingqueue--; this.HandleEvent(idx); - }.bind(this), - function(error){throw error;}); + }.bind(this), // jshint ignore:line + function(error){throw error;}); // jshint ignore:line } else { @@ -710,7 +711,7 @@ FS.prototype.CreateSymlink = function(filename, parentid, symlink) { return this.inodes.length-1; }; -FS.prototype.CreateTextFile = function(filename, parentid, str) { +FS.prototype.CreateTextFile = async function(filename, parentid, str) { // jshint ignore:line const parent_inode = this.inodes[parentid]; if(this.is_forwarder(parent_inode)) { @@ -721,19 +722,20 @@ FS.prototype.CreateTextFile = function(filename, parentid, str) { } var id = this.CreateFile(filename, parentid); var x = this.inodes[id]; - var data = this.inodedata[id] = new Uint8Array(str.length); + var data = new Uint8Array(str.length); x.dirty = true; x.size = str.length; for (var j = 0; j < str.length; j++) { data[j] = str.charCodeAt(j); } + await this.set_data(id, data); // jshint ignore:line return id; -}; +}; // jshint ignore:line /** * @param {Uint8Array} buffer */ -FS.prototype.CreateBinaryFile = function(filename, parentid, buffer) { +FS.prototype.CreateBinaryFile = async function(filename, parentid, buffer) { // jshint ignore:line const parent_inode = this.inodes[parentid]; if(this.is_forwarder(parent_inode)) { @@ -744,12 +746,13 @@ FS.prototype.CreateBinaryFile = function(filename, parentid, buffer) { } var id = this.CreateFile(filename, parentid); var x = this.inodes[id]; - var data = this.inodedata[id] = new Uint8Array(buffer.length); + var data = new Uint8Array(buffer.length); x.dirty = true; data.set(buffer); + await this.set_data(id, data); // jshint ignore:line x.size = buffer.length; return id; -}; +}; // jshint ignore:line FS.prototype.OpenInode = function(id, mode) { @@ -778,7 +781,7 @@ FS.prototype.OpenInode = function(id, mode) { return true; }; -FS.prototype.CloseInode = function(id) { +FS.prototype.CloseInode = async function(id) { // jshint ignore:line //message.Debug("close: " + this.GetFullPath(id)); var inode = this.inodes[id]; if(this.is_forwarder(inode)) @@ -788,15 +791,14 @@ FS.prototype.CloseInode = function(id) { if (inode.status == STATUS_UNLINKED) { //message.Debug("Filesystem: Delete unlinked file"); inode.status = STATUS_INVALID; - delete this.inodedata[id]; - inode.size = 0; + await this.DeleteData(id); // jshint ignore:line } -}; +}; // jshint ignore:line /** * @return {number} 0 if success, or -errno if failured. */ -FS.prototype.Rename = function(olddirid, oldname, newdirid, newname) { +FS.prototype.Rename = async function(olddirid, oldname, newdirid, newname) { // jshint ignore:line // message.Debug("Rename " + oldname + " to " + newname); if ((olddirid == newdirid) && (oldname == newname)) { return 0; @@ -834,8 +836,8 @@ FS.prototype.Rename = function(olddirid, oldname, newdirid, newname) { { // Move inode within the same child filesystem. - const ret = this.follow_fs(olddir) - .Rename(olddir.foreign_id, oldname, newdir.foreign_id, newname); + const ret = await // jshint ignore:line + this.follow_fs(olddir).Rename(olddir.foreign_id, oldname, newdir.foreign_id, newname); if(ret < 0) return ret; } @@ -862,7 +864,7 @@ FS.prototype.Rename = function(olddirid, oldname, newdirid, newname) { const diverted_old_idx = this.divert(olddirid, oldname); const old_real_inode = this.GetInode(idx); - const data = this.Read(diverted_old_idx, 0, old_real_inode.size); + const data = await this.Read(diverted_old_idx, 0, old_real_inode.size); // jshint ignore:line if(this.is_forwarder(newdir)) { @@ -889,10 +891,10 @@ FS.prototype.Rename = function(olddirid, oldname, newdirid, newname) { } // Rewrite data to newly created destination. - this.ChangeSize(idx, old_real_inode.size); + await this.ChangeSize(idx, old_real_inode.size); // jshint ignore:line if(data && data.length) { - this.Write(idx, 0, data.length, data); + await this.Write(idx, 0, data.length, data); // jshint ignore:line } // Move children to newly created destination. @@ -900,13 +902,13 @@ FS.prototype.Rename = function(olddirid, oldname, newdirid, newname) { { for(const child_filename of this.GetChildren(diverted_old_idx)) { - const ret = this.Rename(diverted_old_idx, child_filename, idx, child_filename); + const ret = await this.Rename(diverted_old_idx, child_filename, idx, child_filename); // jshint ignore:line if(ret < 0) return ret; } } // Perform destructive changes only after migration succeeded. - this.DeleteData(diverted_old_idx); + await this.DeleteData(diverted_old_idx); // jshint ignore:line const ret = this.Unlink(olddirid, oldname); if(ret < 0) return ret; } @@ -914,26 +916,26 @@ FS.prototype.Rename = function(olddirid, oldname, newdirid, newname) { this.NotifyListeners(idx, "rename", {oldpath: oldpath}); return 0; -}; +}; // jshint ignore:line -FS.prototype.Write = function(id, offset, count, buffer) { +FS.prototype.Write = async function(id, offset, count, buffer) { // jshint ignore:line this.NotifyListeners(id, 'write'); var inode = this.inodes[id]; if(this.is_forwarder(inode)) { const foreign_id = inode.foreign_id; - this.follow_fs(inode).Write(foreign_id, offset, count, buffer); + await this.follow_fs(inode).Write(foreign_id, offset, count, buffer); // jshint ignore:line return; } inode.dirty = true; - var data = this.inodedata[id]; + var data = await this.get_data(id); // jshint ignore:line if (!data || data.length < (offset+count)) { - this.ChangeSize(id, Math.floor(((offset+count)*3)/2) ); + await this.ChangeSize(id, Math.floor(((offset+count)*3)/2)); // jshint ignore:line inode.size = offset + count; - data = this.inodedata[id]; + data = await this.get_data(id); // jshint ignore:line } else if (inode.size < (offset+count)) { inode.size = offset + count; @@ -942,25 +944,28 @@ FS.prototype.Write = function(id, offset, count, buffer) { { data.set(buffer.subarray(0, count), offset); } -}; + await this.set_data(id, data); // jshint ignore:line +}; // jshint ignore:line -FS.prototype.Read = function(inodeid, offset, count) +FS.prototype.Read = async function(inodeid, offset, count) // jshint ignore:line { const inode = this.inodes[inodeid]; if(this.is_forwarder(inode)) { const foreign_id = inode.foreign_id; - return this.follow_fs(inode).Read(foreign_id, offset, count); + return await this.follow_fs(inode).Read(foreign_id, offset, count); // jshint ignore:line } - else if(!this.inodedata[inodeid]) + + const data = await this.get_data(inodeid); // jshint ignore:line + if(!data) { return null; } else { - return this.inodedata[inodeid].subarray(offset, offset + count); + return data.subarray(offset, offset + count); } -}; +}; // jshint ignore:line FS.prototype.Search = function(parentid, name) { const parent_inode = this.inodes[parentid]; @@ -1136,7 +1141,7 @@ FS.prototype.Unlink = function(parentid, name) { return 0; }; -FS.prototype.DeleteData = function(idx) +FS.prototype.DeleteData = async function(idx) // jshint ignore:line { const inode = this.inodes[idx]; if(this.is_forwarder(inode)) @@ -1146,7 +1151,32 @@ FS.prototype.DeleteData = function(idx) } inode.size = 0; delete this.inodedata[idx]; -}; +}; // jshint ignore:line + +/** + * @private + * @param {number} idx + * @return {Uint8Array} + */ +FS.prototype.get_data = async function(idx) // jshint ignore:line +{ + const inode = this.inodes[idx]; + dbg_assert(inode, `Filesystem get_data: idx ${idx} does not point to an inode`); + + if(this.inodedata[idx]) return this.inodedata[idx]; + return null; +}; // jshint ignore:line + +/** + * @private + * @param {number} idx + * @param {Uint8Array} buffer + */ +FS.prototype.set_data = async function(idx, buffer) // jshint ignore:line +{ + // Current scheme: Save all modified buffers into local inodedata. + this.inodedata[idx] = buffer; +}; // jshint ignore:line /** * @param {number} idx @@ -1166,19 +1196,20 @@ FS.prototype.GetInode = function(idx) return inode; }; -FS.prototype.ChangeSize = function(idx, newsize) +FS.prototype.ChangeSize = async function(idx, newsize) // jshint ignore:line { var inode = this.GetInode(idx); - var temp = this.inodedata[idx]; + var temp = await this.get_data(idx); // jshint ignore:line inode.dirty = true; //message.Debug("change size to: " + newsize); if (newsize == inode.size) return; - var data = this.inodedata[idx] = new Uint8Array(newsize); + var data = new Uint8Array(newsize); + await this.set_data(idx, data); // jshint ignore:line inode.size = newsize; if(!temp) return; var size = Math.min(temp.length, inode.size); data.set(temp.subarray(0, size), 0); -}; +}; // jshint ignore:line FS.prototype.SearchPath = function(path) { //path = path.replace(/\/\//g, "/"); diff --git a/src/browser/starter.js b/src/browser/starter.js index 6dc26e9f..cd3330b9 100644 --- a/src/browser/starter.js +++ b/src/browser/starter.js @@ -1149,6 +1149,8 @@ V86Starter.prototype.mount_fs = function(path, baseurl, basefs, callback) */ V86Starter.prototype.create_file = function(file, data, callback) { + callback = callback || function() {}; + var fs = this.fs9p; if(!fs) @@ -1165,21 +1167,14 @@ V86Starter.prototype.create_file = function(file, data, callback) if(!not_found) { - fs.CreateBinaryFile(filename, parent_id, data); + fs.CreateBinaryFile(filename, parent_id, data) + .then(() => callback(null)); } - - if(callback) + else { setTimeout(function() { - if(not_found) - { - callback(new FileNotFoundError()); - } - else - { - callback(null); - } + callback(new FileNotFoundError()); }, 0); } }; @@ -1216,16 +1211,17 @@ V86Starter.prototype.read_file = function(file, callback) function() { const size = fs.GetInode(id).size; - const data = fs.Read(id, 0, size); - - if(data) + fs.Read(id, 0, size).then(data => { - callback(null, data); - } - else - { - callback(new FileNotFoundError(), null); - } + if(data) + { + callback(null, data); + } + else + { + callback(new FileNotFoundError(), null); + } + }); } ); }