From cdfdb277f69d2543f10fc929d8beaf2fc0b91a91 Mon Sep 17 00:00:00 2001 From: payonel Date: Thu, 22 Jun 2017 16:46:52 -0700 Subject: [PATCH 1/3] replacing yield_all with yield_past to control stack yielding This change should allow popen, pipes, and threads to specify with exactness how many coroutines back they want to yield. This makes popen and threads immune to internal coroutine scenarios Also using a code cleanup provided by @SDPhantom --- .../loot/openos/boot/01_process.lua | 11 ++-- .../opencomputers/loot/openos/lib/pipe.lua | 53 ++++++++++++------- .../opencomputers/loot/openos/lib/thread.lua | 4 +- 3 files changed, 42 insertions(+), 26 deletions(-) diff --git a/src/main/resources/assets/opencomputers/loot/openos/boot/01_process.lua b/src/main/resources/assets/opencomputers/loot/openos/boot/01_process.lua index 9bab80513..8d1929bbc 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/boot/01_process.lua +++ b/src/main/resources/assets/opencomputers/loot/openos/boot/01_process.lua @@ -6,7 +6,11 @@ local _coroutine = coroutine -- real coroutine backend _G.coroutine = setmetatable( { resume = function(co, ...) - return assert(process.info(co), "thread has no proc").data.coroutine_handler.resume(co, ...) + local proc = process.info(co) + -- proc is nil if the process closed, natural resume will likely complain the coroutine is dead + -- but if proc is dead and an aborted coroutine is alive, it doesn't have any proc data like stack info + -- if the user really wants to resume it, let them + return (proc and proc.data.coroutine_handler.resume or _coroutine.resume)(co, ...) end }, { @@ -51,10 +55,7 @@ end _coroutine.wrap = function(f) local thread = coroutine.create(f) return function(...) - local result_pack = table.pack(coroutine.resume(thread, ...)) - local result, reason = result_pack[1], result_pack[2] - assert(result, reason) - return select(2, table.unpack(result_pack)) + return select(2, coroutine.resume(thread, ...)) end end diff --git a/src/main/resources/assets/opencomputers/loot/openos/lib/pipe.lua b/src/main/resources/assets/opencomputers/loot/openos/lib/pipe.lua index 4f16688cd..80904f055 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/lib/pipe.lua +++ b/src/main/resources/assets/opencomputers/loot/openos/lib/pipe.lua @@ -22,17 +22,18 @@ function pipe.createCoroutineStack(root, env, name) function pco.yield(...) return _root_co.yield(nil, ...) end - function pco.yield_all(...) - return _root_co.yield(true, ...) + function pco.yield_past(co, ...) + return _root_co.yield(co, ...) end function pco.resume(co, ...) checkArg(1, co, "thread") local args = table.pack(...) while true do -- for consecutive sysyields local result = table.pack(_root_co.resume(co, table.unpack(args, 1, args.n))) + local target = result[2] == true and pco.root or result[2] if not result[1] or _root_co.status(co) == "dead" then return table.unpack(result, 1, result.n) - elseif result[2] and pco.root ~= co then + elseif target and target ~= co then args = table.pack(_root_co.yield(table.unpack(result, 2, result.n))) else return true, table.unpack(result, 3, result.n) @@ -70,7 +71,8 @@ local pipe_stream = return self end -- not reading, it is requesting a yield - result = table.pack(coroutine.yield_all(table.unpack(result, 2, result.n))) + -- yield_past(true) will exit this coroutine stack + result = table.pack(coroutine.yield_past(true, table.unpack(result, 2, result.n))) result = table.pack(coroutine.resume(self.next, table.unpack(result, 1, result.n))) -- the request was for an event end end, @@ -113,7 +115,7 @@ local pipe_stream = -- natural yield (i.e. for events). To differentiate this yield from natural -- yields we set read_mode here, which the pipe_stream write detects self.read_mode = true - coroutine.yield_all() + coroutine.yield_past(self.next) -- next is the first croutine in this stack self.read_mode = false end local result = string.sub(self.buffer, 1, n) @@ -158,16 +160,24 @@ end local chain_stream = { - read = function(self, value) + read = function(self, value, ...) if self.io_stream.closed then return nil end - -- handler is currently on yield all [else we wouldn't have control here] - local read_ok, ret = self.pco.resume(self.pco.root, value) - -- ret can be non string when a process ends - ret = type(ret) == "string" and ret or nil - return select(read_ok and 2 or 1, nil, ret) + -- wake up prog + self.ready = false -- the pipe proc sets this true when ios completes + local ret = table.pack(coroutine.resume(self.pco.root, value, ...)) + if coroutine.status(self.pco.root) == "dead" then + return nil + elseif not ret[1] then + return table.unpack(ret, 1, ret.n) + end + if not self.ready then + -- prog yielded back without writing/reading + return self:read(coroutine.yield()) + end + return ret[2] end, write = function(self, ...) - return self:read(table.concat({...})) + return self:read(...) end, close = function(self) self.io_stream:close() @@ -181,8 +191,8 @@ function pipe.popen(prog, mode, env) end local r = mode == "r" - local key = r and "read" or "write" + local chain = {} -- to simplify the code - shell.execute is run within a function to pass (prog, env) -- if cmd_proc where to come second (mode=="w") then the pipe_proc would have to pass -- the starting args. which is possible, just more complicated @@ -194,22 +204,27 @@ function pipe.popen(prog, mode, env) -- the stream needs its own process for io local pipe_proc = process.load(function() local n = r and 0 or "" + local key = r and "read" or "write" local ios = stream.io_stream while not ios.closed do - n = coroutine.yield_all(ios[key](ios, n)) + -- read from pipe + local ret = table.pack(ios[key](ios, n)) + stream.ready = true + -- yield outside the chain now + n = coroutine.yield_past(chain[1], table.unpack(ret, 1, ret.n)) end end, nil, nil, "pipe_handler") - local pipe_index = r and 2 or 1 - local cmd_index = r and 1 or 2 - local chain = {[cmd_index]=cmd_proc, [pipe_index]=pipe_proc} + chain[r and 1 or 2] = cmd_proc + chain[r and 2 or 1] = pipe_proc -- link the cmd and pipe proc io pipe.buildPipeChain(chain) - local cmd_stack = process.info(chain[1]).data.coroutine_handler + local cmd_data = process.info(chain[1]).data + local cmd_stack = cmd_data.coroutine_handler -- store handle to io_stream from easy access later - stream.io_stream = process.info(chain[1]).data.io[1].stream + stream.io_stream = cmd_data.io[1].stream stream.pco = cmd_stack -- popen commands start out running, like threads diff --git a/src/main/resources/assets/opencomputers/loot/openos/lib/thread.lua b/src/main/resources/assets/opencomputers/loot/openos/lib/thread.lua index f602e7b77..317b28a7e 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/lib/thread.lua +++ b/src/main/resources/assets/opencomputers/loot/openos/lib/thread.lua @@ -213,11 +213,11 @@ function thread.create(fp, ...) function mt.process.data.pull(_, timeout) mt.register(timeout) - -- yield_all will yield this pco stack + -- yield_past(root) will yield until out of this thread -- the callback will resume this stack local event_data repeat - event_data = table.pack(t.pco.yield_all(timeout)) + event_data = table.pack(t.pco.yield_past(t.pco.root, timeout)) -- during sleep, we may have been suspended until t:status() ~= "suspended" return table.unpack(event_data, 1, event_data.n) From f9f37897296ec5699718f9d723832ef63774e46f Mon Sep 17 00:00:00 2001 From: SDPhantom Date: Fri, 23 Jun 2017 15:52:47 -0700 Subject: [PATCH 2/3] Optimized sleep.lua interval parsing (#2436) * Optimized sleep.lua interval parsing Modified regex to use captures, consolidated invalid interval checks. Note: tonumber(nil) safely returns nil * Update sleep.lua Updated pattern matching --- .../assets/opencomputers/loot/openos/bin/sleep.lua | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/main/resources/assets/opencomputers/loot/openos/bin/sleep.lua b/src/main/resources/assets/opencomputers/loot/openos/bin/sleep.lua index 95701a159..43e2d5222 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/bin/sleep.lua +++ b/src/main/resources/assets/opencomputers/loot/openos/bin/sleep.lua @@ -39,23 +39,15 @@ end local total_time = 0 for _,v in ipairs(args) do - local interval = v:match('^%d+%.?%d*[smhd]?$') - if not interval then - help(v) - return 1 - end - - local time_type = interval:match('[smhd]') or '' - interval = interval:sub(1, -#time_type-1) + local interval, time_type = v:match('^([%d%.]+)([smhd]?)$') interval = tonumber(interval) - if interval < 0 then + if not interval or interval < 0 then help(v) return 1 end - interval = time_type_multiplier(time_type) * interval - total_time = total_time + interval + total_time = total_time + time_type_multiplier(time_type) * interval end os.sleep(total_time) From 90fdcc19c541b5bfed2a4e3b1c93cb5172828051 Mon Sep 17 00:00:00 2001 From: payonel Date: Sat, 24 Jun 2017 09:04:11 -0700 Subject: [PATCH 3/3] keep env clean with intercepted loads also remove unused vars and some light code clean up moving some methods out of tty into delay loaded space for memory fix print from overwriting vbuf settings on stdout fix regression for expanding ${VAR} closes #2434 --- .../opencomputers/loot/openos/bin/rc.lua | 13 ++--- .../loot/openos/boot/00_base.lua | 3 +- .../loot/openos/boot/01_process.lua | 26 +++++----- .../loot/openos/lib/filesystem.lua | 8 +-- .../opencomputers/loot/openos/lib/sh.lua | 2 +- .../opencomputers/loot/openos/lib/text.lua | 4 +- .../opencomputers/loot/openos/lib/tty.lua | 49 ++----------------- .../loot/openos/opt/core/full_tty.lua | 41 +++++++++++++++- .../opencomputers/loot/openos/usr/man/sh | 7 ++- 9 files changed, 76 insertions(+), 77 deletions(-) diff --git a/src/main/resources/assets/opencomputers/loot/openos/bin/rc.lua b/src/main/resources/assets/opencomputers/loot/openos/bin/rc.lua index 488553626..ebb35364f 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/bin/rc.lua +++ b/src/main/resources/assets/opencomputers/loot/openos/bin/rc.lua @@ -1,6 +1,5 @@ local rc = require("rc") local fs = require("filesystem") -local shell = require("shell") local function loadConfig() local env = {} @@ -117,9 +116,7 @@ local function allRunCommand(cmd, ...) return results end -local args = table.pack(...) - -if #args == 0 then +if select("#", ...) == 0 then local results,reason = allRunCommand("start") if not results then local msg = "rc failed to start:"..tostring(reason) @@ -127,16 +124,16 @@ if #args == 0 then require("event").onError(msg) return end - for name, result in pairs(results) do + for _, result in pairs(results) do local ok, reason = table.unpack(result) if not ok then - io.stderr:write(reason .. "\n") + io.stderr:write(reason, "\n") end end else - local result, reason = runCommand(table.unpack(args)) + local result, reason = runCommand(...) if not result then - io.stderr:write(reason .. "\n") + io.stderr:write(reason, "\n") return 1 end end diff --git a/src/main/resources/assets/opencomputers/loot/openos/boot/00_base.lua b/src/main/resources/assets/opencomputers/loot/openos/boot/00_base.lua index 50fe336b7..76c4c04ae 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/boot/00_base.lua +++ b/src/main/resources/assets/opencomputers/loot/openos/boot/00_base.lua @@ -32,6 +32,7 @@ end function print(...) local args = table.pack(...) local stdout = io.stdout + local old_mode, old_size = stdout:setvbuf() stdout:setvbuf("line") local pre = "" for i = 1, args.n do @@ -39,6 +40,6 @@ function print(...) pre = "\t" end stdout:write("\n") - stdout:setvbuf("no") + stdout:setvbuf(old_mode, old_size) stdout:flush() end diff --git a/src/main/resources/assets/opencomputers/loot/openos/boot/01_process.lua b/src/main/resources/assets/opencomputers/loot/openos/boot/01_process.lua index 8d1929bbc..9ae2e27be 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/boot/01_process.lua +++ b/src/main/resources/assets/opencomputers/loot/openos/boot/01_process.lua @@ -26,18 +26,22 @@ local kernel_load = _G.load local intercept_load intercept_load = function(source, label, mode, env) if env then - local loader = setmetatable( - { - env = env, - load = intercept_load, - }, - {__call = function(tbl, _source, _label, _mode, _env) - return tbl.load(_source, _label, _mode, _env or tbl.env) - end}) - if env.load and (type(env.load) ~= "table" or env.load.load ~= intercept_load) then - loader.load = env.load + local prev_load = env.load or intercept_load + local env_mt = getmetatable(env) or {} + local env_mt_index = env_mt.__index + env_mt.__index = function(tbl, key) + if key == "load" then + return function(_source, _label, _mode, _env) + return prev_load(_source, _label, _mode, _env or env) + end + elseif type(env_mt_index) == "table" then + return env_mt_index[key] + elseif env_mt_index then + return env_mt_index(tbl, key) + end + return nil end - env.load = loader + setmetatable(env, env_mt) end return kernel_load(source, label, mode, env or process.info().env) end diff --git a/src/main/resources/assets/opencomputers/loot/openos/lib/filesystem.lua b/src/main/resources/assets/opencomputers/loot/openos/lib/filesystem.lua index 2e4ab54a0..2346e7ae7 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/lib/filesystem.lua +++ b/src/main/resources/assets/opencomputers/loot/openos/lib/filesystem.lua @@ -136,7 +136,7 @@ function filesystem.concat(...) end function filesystem.get(path) - local node, rest = findNode(path) + local node = findNode(path) if node.fs then local proxy = node.fs path = "" @@ -155,7 +155,7 @@ end function filesystem.realPath(path) checkArg(1, path, "string") - local node, rest, vnode, vrest = findNode(path, false, true) + local node, rest = findNode(path, false, true) if not node then return nil, rest end local parts = {rest or nil} repeat @@ -199,7 +199,7 @@ function filesystem.link(target, linkpath) return nil, "not a directory" end - local node, rest, vnode, vrest = findNode(linkpath_real, true) + local _, _, vnode, _ = findNode(linkpath_real, true) vnode.links[filesystem.name(linkpath)] = target return true end @@ -235,7 +235,7 @@ function filesystem.mount(fs, path) if fstab[real] then return nil, "another filesystem is already mounted here" end - for path,node in pairs(fstab) do + for _,node in pairs(fstab) do if node.fs.address == fs.address then fsnode = node break diff --git a/src/main/resources/assets/opencomputers/loot/openos/lib/sh.lua b/src/main/resources/assets/opencomputers/loot/openos/lib/sh.lua index 7ff6e33d4..a17cf0ddd 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/lib/sh.lua +++ b/src/main/resources/assets/opencomputers/loot/openos/lib/sh.lua @@ -133,7 +133,7 @@ function sh.expand(value) end) :gsub("%${(.*)}", function(key) if sh.internal.isIdentifier(key) then - return sh.internal.expandKey(key) + return os.getenv(key) or '' end io.stderr:write("${" .. key .. "}: bad substitution\n") os.exit(1) diff --git a/src/main/resources/assets/opencomputers/loot/openos/lib/text.lua b/src/main/resources/assets/opencomputers/loot/openos/lib/text.lua index 1a0d8fd0e..764ba408f 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/lib/text.lua +++ b/src/main/resources/assets/opencomputers/loot/openos/lib/text.lua @@ -42,10 +42,10 @@ function text.wrap(value, width, maxWidth) end function text.wrappedLines(value, width, maxWidth) - local line, nl + local line return function() if value then - line, value, nl = text.wrap(value, width, maxWidth) + line, value = text.wrap(value, width, maxWidth) return line end end diff --git a/src/main/resources/assets/opencomputers/loot/openos/lib/tty.lua b/src/main/resources/assets/opencomputers/loot/openos/lib/tty.lua index 5afbe46b7..8d708282b 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/lib/tty.lua +++ b/src/main/resources/assets/opencomputers/loot/openos/lib/tty.lua @@ -42,42 +42,7 @@ local function read_history(handler, cursor, change) end end -local function tab_handler(handler, cursor) - local hints = handler.hint - if not hints then return end - local main_kb = tty.keyboard() - -- tty may not have a keyboard - -- in which case, we shouldn't be handling tab events - if not main_kb then - return - end - if not handler.cache then - handler.cache = type(hints) == "table" and hints or hints(cursor.data, cursor.index + 1) or {} - handler.cache.i = -1 - end - - local cache = handler.cache - - if #cache == 1 and cache.i == 0 then - -- there was only one solution, and the user is asking for the next - handler.cache = hints(cache[1], cursor.index + 1) - if not handler.cache then return end - handler.cache.i = -1 - cache = handler.cache - end - - local change = kb.isShiftDown(main_kb) and -1 or 1 - cache.i = (cache.i + change) % math.max(#cache, 1) - local next = cache[cache.i + 1] - if next then - local tail = unicode.len(cursor.data) - cursor.index - cursor:clear() - cursor:update(next) - cursor:move(-tail) - end -end - -local function key_down_handler(handler, cursor, char, code) +function tty.key_down_handler(handler, cursor, char, code) local c = false local backup_cache = handler.cache handler.cache = nil @@ -86,7 +51,7 @@ local function key_down_handler(handler, cursor, char, code) return --close elseif code == keys.tab then handler.cache = backup_cache - tab_handler(handler, cursor) + tty.on_tab(handler, cursor) elseif code == keys.enter or code == keys.numpadenter then cursor:move(math.huge) cursor:draw("\n") @@ -307,11 +272,6 @@ function tty.read(handler, cursor) checkArg(2, cursor, "table", "nil") handler.index = 0 - handler.touch = handler.touch or "touch_handler" - handler.drag = handler.drag or "touch_handler" - handler.clipboard = handler.clipboard or "clipboard_handler" - handler.key_down = handler.key_down or key_down_handler - cursor = cursor or tty.internal.build_vertical_reader() while true do @@ -329,11 +289,8 @@ function tty.read(handler, cursor) tty.drawText("^C\n") return false elseif address == main_kb or address == main_sc then - local handler_method = handler[name] + local handler_method = handler[name] or tty[name .. "_handler"] if handler_method then - if type(handler_method) == "string" then -- special hack to delay loading tty stuff - handler_method = tty[handler_method] - end -- nil to end (close) -- false to ignore -- true-thy updates cursor diff --git a/src/main/resources/assets/opencomputers/loot/openos/opt/core/full_tty.lua b/src/main/resources/assets/opencomputers/loot/openos/opt/core/full_tty.lua index 7dcd499f1..67ab6b9e1 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/opt/core/full_tty.lua +++ b/src/main/resources/assets/opencomputers/loot/openos/opt/core/full_tty.lua @@ -1,7 +1,8 @@ local tty = require("tty") local unicode = require("unicode") +local kb = require("keyboard") -function tty.touch_handler(handler, cursor, gx, gy) +function tty.touch_handler(_, cursor, gx, gy) if cursor.data == "" then return false end @@ -33,8 +34,9 @@ function tty.touch_handler(handler, cursor, gx, gy) end return false -- no further cursor update end +tty.drag_handler = tty.touch_handler -function tty.clipboard_handler(handler, cursor, char, code) +function tty.clipboard_handler(handler, _, char, _) handler.cache = nil local first_line, end_index = char:find("\13?\10") if first_line then @@ -49,3 +51,38 @@ function tty.clipboard_handler(handler, cursor, char, code) return char end +function tty.on_tab(handler, cursor) + local hints = handler.hint + if not hints then return end + local main_kb = tty.keyboard() + -- tty may not have a keyboard + -- in which case, we shouldn't be handling tab events + if not main_kb then + return + end + if not handler.cache then + handler.cache = type(hints) == "table" and hints or hints(cursor.data, cursor.index + 1) or {} + handler.cache.i = -1 + end + + local cache = handler.cache + + if #cache == 1 and cache.i == 0 then + -- there was only one solution, and the user is asking for the next + handler.cache = hints(cache[1], cursor.index + 1) + if not handler.cache then return end + handler.cache.i = -1 + cache = handler.cache + end + + local change = kb.isShiftDown(main_kb) and -1 or 1 + cache.i = (cache.i + change) % math.max(#cache, 1) + local next = cache[cache.i + 1] + if next then + local tail = unicode.len(cursor.data) - cursor.index + cursor:clear() + cursor:update(next) + cursor:move(-tail) + end +end + diff --git a/src/main/resources/assets/opencomputers/loot/openos/usr/man/sh b/src/main/resources/assets/opencomputers/loot/openos/usr/man/sh index 9341521e6..bc6607c9f 100644 --- a/src/main/resources/assets/opencomputers/loot/openos/usr/man/sh +++ b/src/main/resources/assets/opencomputers/loot/openos/usr/man/sh @@ -13,17 +13,20 @@ DESCRIPTION Single quotes also suppress variable expansion. Per default, expressions like `$NAME` and `${NAME}` are expanded using environment variables (also accessible via the `os.getenv` method). - Basic globbing is supported, i.e. '*' and '?' are expanded approriately. For example: + Globbing is supported, i.e. '*' and '?' are expanded approriately. For example: ls b?n/ will list all files in `/bin/` (and, if it exists `/ban` and so on). cp /bin/* /usr/bin/ will copy all files from `/bin` to `/usr/bin`. - The shell provides basic redirects and piping: + The shell provides redirects and piping: cat f > f2 copies the contents of file `f` to `f2`, for example. echo 'this is a "test"' >> f2 will append the string 'this is a "test"' to the file `f2`. + 2>/dev/null ./some_program_with_errors + will redirect all stderr to /dev/null [i.e. supress errors]. + This example also demonstrates redirects can go at the front Redirects can be combined: cat < f >> f2