Re: [BUG] LuaSocket is not thread safe

[Philippe Verdy <verdy_p@...>]

I do agree, such a small arrayshould be a simple constant static declaration, not a function (and no need to use a boolean static tracker to see if it's initialized), or should be called only from the static initializer "b64setup" of the library (once for all threads) which should not run multiple times (such library initializer should have an exclusive mutex, which should be part of the "require" feature when it loads modules from concurrent threads).

However Lua does not natively uses truly preemptive concurrent threads (Posix like) and its core source just uses cooperative threads that never run concurrently (only when they yield), so this bug is not observed with the base implementation where all Lua threads (states) are running in the same thread (other threads are not used directly by Lua code but only by internal implemention for OS integration of I/O or socket/networking, notably listeners that then distribute the work loads and queue/serialize them for execution in the main thread where cooperative Lua threads are running).

Because of this, most Lua code is not thread-safe and it would require extensive work to make it so and allow the standard implementation being fully aware of preemptive concurrent threads as there's still no standard API for using mutexes, critical sections or thread-local storage, even if this API would be a NO-OP for the basic cooperative implementation.

For now I've not seen any Lua implementation working successfully to run Lua states in concurrent threads. If this exists, there is necearrily numerous patches everywhere in the source base to add many declarations and specific code to control concurrent accesses and serialize them when needed (with mutexes, critical sections and some variables in thread local storage when we want to limit the performance cost of mutexes and serialization). As well, making Lua using fully concurrent threads can expose it to severe security problems (e.g. Spectre/Meltdown issues) and modifications in the compiler chain (e.g. using "Retpoline" workarounds): this is also a complex issue in other scripting languages (notably _javascript_ and Java) and simpler to manage if concurrency is done only with full processes (because this problem should be fixed at OS-level), for example when using pools of worker processes to run multiple Lua instances on application servers: concurrent threads would be performing better than concurrent processes. But the basic cooperative threads of Lua are much easier to secure.

So your problem is actually: how do we control when b64setup() can run, and more generally how can "require" work concurrently and safely in a true multithreading environment to initialize the loaded modules? May be we don't need to patch this code and this should be better handled at a higher level in Lua's core implementation of "require"...

Le sam. 9 mars 2019 à 13:21, Viacheslav Usov <via.usov@gmail.com> a écrit :

With this grand opening title, I would like to attract attention to the following problem in mime.c:.

Function luaopen_mime_core that is called when the mime module is required, calls function b64setup, which initialises a static (global) array b64unbase. It does this as follows:

static void b64setup(UC *unbase)

{

    int i;

    for (i = 0; i <= 255; i++) unbase[i] = (UC) 255;

    for (i = 0; i < 64; i++) unbase[b64base[i]] = (UC) i;

    unbase['='] = 0;

}

Observe that the first loop fills the entire array with byte value 255, then certain elements within the array are overwritten with some other values. Those other values are expected in mime.unb64.

In an application that hosts multiple Lua states this function will be called multiple times if code in multiple states requires mime. If the states run in concurrent threads, these calls may execute concurrently, too, and then can also run concurrently with the code that calls mime.unb64, which may see the incorrect byte value 255 set up by the first loop. There is no simple way to reproduce this except by using a debugger to freeze and thaw threads in the right places.

There are multiple ways to fix that, but perhaps it is best to initialise those arrays statically, because running the init code multiple times does not make much sense.

Cheers,

V.