Create the sandbox

[numberZero]

• Providing a secure execution environment in which to parse and run the code (checking for binary input to loadstring, turning off JIT, setting up the timeout hook, building an appropriate environment).
• Providing the core internal APIs that only apply within the scripting environment (string, math, table, maybe os, etc.).
• Providing some basic communication APIs (print, mem, something like that).

Parent issue: minetest-mods/mesecons#283

[Hawk777]

Thoughts on my proposed API?

[Hawk777]

Also, what’s the plan for choice of license? It looks like Mesecons is Lesser GPL 3, though COPYING.txt also makes reference to the Library GPL, which is a different and older license. I assume we don’t have to care about the media portion since, as a non-player-facing mod, we won’t ever have any media in libluabox. Other than perhaps documentation, I guess?

[numberZero]

Thoughts on my proposed API?

“You pass a function, and a list of functions, into a function, to get an object that, indeed, is a function itself.” Too many functions involved in a single function call.

Also, the word about pcall is confusing... is this sandbox or what?

So, as I’m back, I started writing the sandbox myself; that seems to require my full attention. Also note that I moved your branch to new name; the documentation you wrote is worth keeping, I need to read it more thoroughly.

[Hawk777]

On October 25, 2017 10:37:43 AM PDT, Vitaliy notifications@github.com wrote:

Thoughts on my proposed API?

“You pass a function, and a list of functions, into a function, to get an object that, indeed, is a function itself.” Too many functions involved in a single function call.

Also, the word about pcall is confusing... is this sandbox or what?

So, as I’m back, I started writing the sandbox myself; that seems to require my full attention. Also note that I moved your branch to new name; the documentation you wrote is worth keeping, I need to read it more thoroughly.

I will reply in more detail this evening because I don’t have proper Github access here, but I already wrote all the code. It just isn’t committed yet because I was waiting for comments on the API first.

-- Christopher Head

[numberZero]

mods which run the same code frequently will construct a single program object, cache it (perhaps in a weak table), and reuse it for multiple invocations.

Well, can hardly imagine such mods and items. Maybe something like a high-bandwidth user-programmable digiline router... ;) Anyway, thank you for mentioning weak tables; using them, caching may be done internally and transparently (and more efficiently).

[numberZero]

I already wrote all the code. I was waiting for comments

Well, that sounds strange. Still, can you show the code?

this evening

USA? Sorry but I’ll sleep until your late night.

[Hawk777]

On October 25, 2017 11:15:14 AM PDT, Vitaliy notifications@github.com wrote:

I already wrote all the code. I was waiting for comments

Well, that sounds strange. Still, can you show the code?

this evening

USA? Sorry but I’ll sleep until your late night.

North American Pacific time (UTC-7 currently). I didn’t want to share the code because I thought it made no sense to do that until the API was designed. If you want me to, I can.

-- Christopher Head

[Hawk777]

“You pass a function, and a list of functions, into a function, to get an object that, indeed, is a function itself.” Too many functions involved in a single function call.

I guess you’re talking about libluabox.create_program? It’s meant to be the constructor for a program object. Actually there’s only one set of functions involved: src is a string not a function, env_builders is a list of functions, and the return value is a table not a function. What kind of refactoring would you suggest? If you find that too many parameters, would you like an add_env_builder function instead which can add environment builders one by one?

Also, the word about pcall is confusing... is this sandbox or what?

Do you mean the bullet point in the “Guidelines for environment builders” section? That’s a guideline for writing environment builders, not for writing user scripts. User scripts don’t have access to pcall at all. If you, as a mod author, write a function and add that function to the user script’s environment, then you should not swallow errors inside that function using pcall. I thought it was fairly clear that those bullet points were for people writing environment builders (i.e. mod authors), not players writing scripts?

Well, can hardly imagine such mods and items. Maybe something like a high-bandwidth user-programmable digiline router... ;)

I would have thought a Mesecons Luacontroller, running an interrupt or more per second and perhaps talking Digilines to a few peripherals, could easily get to the point where performance might matter, especially if you have a few dozen Luacontrollers doing that rather than just one. That doesn’t seem like an unreasonable kind of circuit for a player to build.

Anyway, thank you for mentioning weak tables; using them, caching may be done internally and transparently (and more efficiently).

How would you do caching internally in libluabox? You don’t know where the program comes from. In the Luacontroller case the cache should be keyed by node position, but then it needs to be invalidated when the player sets a new script or when the node is dug. If it’s a handheld computer then the logic is completely different and it doesn’t even have a node position. I think caching belongs in the individual consuming mods.

Well, that sounds strange. Still, can you show the code?

I didn’t want to influence design decisions by sharing the code (IMO design decisions should be mostly settled before writing code, so that you code something useful instead of something easy to build), but I also didn’t feel like waiting before writing the code. I thought keeping it local would accomplish that, but apparently it led to confusion as to who would actually write the code; I thought I was doing that but then you wrote some yourself. Anyway, b447c153a30e has it.

[Hawk777]

By the way, I likely won’t be able to reply to any further comments until next week.

[numberZero]

would you like an add_env_builder function instead which can add environment builders one by one?

I don’t like the whole idea of environment builders. What’s wrong with simple immediate-mode API?

Do you mean the bullet point in the “Guidelines for environment builders” section?

No, I mean create_program: calling mods probably want to use pcall or xpcall in order to report this error to the player usefully and avoid crashing the world.

How would you do caching internally in libluabox? You don’t know where the program comes from.

But you know what program comes; the code may be used as the key.

In the Luacontroller case the cache should be keyed by node position

And then the LuaC is pushed by a piston... No, really, caching can be done on the client (i.e. mesecons) side, but it would be way more problematic (although more compact probably).

I didn’t want to influence design decisions by sharing the code (IMO design decisions should be mostly settled before writing code, so that you code something useful instead of something easy to build)

Correct.

but I also didn’t feel like waiting before writing the code.

...that looks so familiar... it’s LGPL’d, that should not be forgotten.

[Hawk777]

I don’t like the whole idea of environment builders. What’s wrong with simple immediate-mode API?

I’m not sure if the code you have in master is meant to let you cache parsed function objects or not (if it is, then it has some bugs which I can report later if you decide to keep your code rather than mine). Mine is. In such a situation:

• To prevent inappropriate persistence, you need to wipe and refill the environment on every run.
• Some of the environment should come from libluabox.
• Some of the environment should come from the consuming mod.
• Some of the environment should be different on each run (e.g. event).

It seemed to me that environment builders was the obvious way to represent that. You provide them when you construct the sandbox. The consuming mod doesn’t have to remember to clear the environment and manually rebuild it before each invocation; libluabox remembers to do that. But environment builders provide a hook where the consuming mod can stick in whatever extra bits of environment that it wants to. The way you’ve written master, the consuming mod has to remember to clear sandbox.env, then call sandbox:include_core(), then tack on some more items. That seems more confusing than providing environment builders, but I guess both ways work.

Side note, why does every consuming mod for your libluabox have to provide its own safe string and os libraries?

No, I mean create_program: calling mods probably want to use pcall or xpcall in order to report this error to the player usefully and avoid crashing the world.

pcall also has to be used when running the program. Lua has an error reporting mechanism and I decided to use it to report errors, such as parse errors when parsing the code. I don’t see why this is weird? I’m just documenting which functions can raise errors and which can’t, and also pointing out the fact that if a mod lets a reported error propagate to Minetest core rather than handling it then the world explodes.

But you know what program comes; the code may be used as the key.

Yes, that works if you rebuild the environment on each run rather than storing any state, which you do. However, be aware that Lua uses only the first few characters of a string for hashing (I would cite a source but I can’t find it in the couple of minutes I have available to write this comment; let me know if you want it and I'll find it later), so if you have many blocks of code, you’ll end up having the native Lua code doing a table lookup by iterating through many large strings and equality comparing them one by one. Also, the strings are probably quite large, and I thought it was better not to keep them in memory. But meh. Probably not a big deal.

...that looks so familiar... it’s LGPL’d, that should not be forgotten.

I wrote my code. I didn’t copy and paste it from Mesecons. Of course I’ve already read Mesecons, so was influenced, and also there’s really only one way to do many things in a Lua sandbox. It doesn’t look that similar, I didn’t think.. But I don’t really care, because I asked what license to use earlier, and I’m fine with LGPL, so it doesn’t matter whether I copied or recoded.

[numberZero]

Side note, why does every consuming mod for your libluabox have to provide its own safe string and os libraries?

Because the code I pushed was in-progress. You can recheck; what I pushed last time is more appropriate, although still unfinished.

Lua uses only the first few characters of a string for hashing

Not exactly: https://github.com/minetest/minetest/blob/master/lib/lua/src/lstring.c#L77 http://repo.or.cz/luajit-2.0.git/blob/HEAD:/src/lj_str.c#l100 It uses a few characters (not only first ones) and length.

equality comparing them one by one.

Even then, comparison is done by identity, not by content, as there can’t be two distinct strings with equal content in Lua.

[numberZero]

Er, I see that you assume the sandbox to be long-living. I assume it to be short-living, generally. That’s the difference.

[Hawk777]

Because the code I pushed was in-progress. You can recheck; what I pushed last time is more appropriate, although still unfinished.

OK, fair enough.

Not exactly

My mistake; I was basing this on a message in the mailing list archives which I read some time ago and just remembered as well as I could.

Even then, comparison is done by identity, not by content, as there can’t be two distinct strings with equal content in Lua.

OK then.

Er, I see that you assume the sandbox to be long-living. I assume it to be short-living, generally. That’s the difference.

Yes, I think this correlates with my assumption that client mods would do caching versus your assumption that libluabox would do caching. If the client mod is to do the caching, it has to have something to cache; that something is a program object. If libluabox does the caching, whatever thing you call “a sandbox” doesn’t really care whether it lives for a long or a short time, since the performance benefit from keeping something for a long time (caching) is done inside libluabox itself and is orthogonal to the objects used to access it.

You own this repo; I don’t. You are also a member of the minetest-mods Github group; I am not. I think the most sensible thing right now is for you to decide (perhaps in consultation with some other mod authors, as I see you have pinged a few people on minetest-mods/mesecons#387) and decide which variant you want to keep. I’ll answer any questions you might have about my code, of course, but I see no reason to further develop either code or design until that decision is made. Afterwards, if you choose your implementation, I’ll be happy to look through it and try to find bugs, and regardless of choice of implementation, I can also help with PRs on Mesecons and Pipeworks.

[Hawk777]

Hey, just wondering if there’s been any progress in the last month? Certainly not urgent, just was wondering.

[numberZero]

No. I’m busy IRL and in the core; others seem to be busy too. BTW, I’m not a member of minetest-mods. I have access to mesecons only.

On topic:

whatever thing you call “a sandbox” doesn’t really care whether it lives for a long or a short time

It doesn’t, but APIs are usually different for persistent and temporary objects. E.g. if you construct the object right before using it, you can’t forget to reset it as it is clean already; you can’t forget whether you initialized the environment somewhere already as you know you didn’t, and so on.

So, I prefer my API. Environment builders might be useful if you would be able to prepare their list at mod initialization, but that’s problematic as they may need arguments. There may be some helper functions for that, though; you may even wrap your API around mine, that’s totally possible (including caching, as as long as that all remains in the modpack it is allowed to access all the “private” properties).

Also note that the reason I made this a modpack is to move all the (unwritten yet) helpers, compatibility (with mesecons, digilines, etc.) layers, etc. to separate mods to avoid any dependencies from the core.

P.S. Remember that many modders like to do things the wrong way ;) Don’t let them to mess with such complex things as caching; memory leak is the least trouble they may make doing that =)

[Hawk777]

Ah, my apologies about the group membership. Somehow I had gotten the impression you were.

Anyway, now that you explain it that way, I think I do like your plan quite a lot. I had initially thought it seemed a bit crazy to use the entire source code as a cache key, but I suppose most user scripts aren’t really going to be that big; compared to the memory consumption of an entire Minetest world loaded into a server, the scripts will be a tiny fraction. As soon as that idea works, the rest just falls out of it nicely—the only reason I wanted clients to manage lifetime of long-lived objects was for caching, and you’ve just provided a zero-effort way to do caching instead.

As for the modpack thing, it seemed pretty easy to write code that could interact with other mods if present without requiring them (relying on late binding and checking for the existence of global variables), but the split is also very reasonable IMO. Perhaps even tidier for dependency resolution.