koreader binary on every platform

[pazos]

Intent

A koreader binary on each supported platform

The binary replaces ./reader.lua on each platform shell scripts.

Purpose

1. Keep logs on a ring buffer instead of redirecting stdout to file
2. Run the luajit instance on a thread and wait for it to finish
3. Dump ring buffer to crash.log when the lua vm dies unexpectedly
4. Dump ring buffer to koreader.log on user demand

Unlocks:

Run the native input loop on a different thread
Many more things I cannot tell yet :)

Notes

I'm not interested in multiple lua threads.

[Frenzie]

If you put that in AI you get something that looks like a pretty plausible starting point as far as it goes. I think what it wrote should work on Android too regarding phtread, but it's still worth keeping in mind that Android support for that is quite limited.

(Also I think you're generally supposed to use pthread_mutex_init, but minor details like that aside.)

I have no real opinion one way or the other, though I do wonder if it might be harder to debug? Running luajit directly provides easy access to various helpful arguments.

#include <lua.h>
#include <lualib.h>
#include <lauxlib.h>
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define RING_BUFFER_SIZE 1000
#define MAX_LOG_LENGTH 256

typedef struct {
    char messages[RING_BUFFER_SIZE][MAX_LOG_LENGTH];
    int head;
    int tail;
    int count;
} RingBuffer;

RingBuffer ring_buffer = {0};
pthread_mutex_t buffer_mutex = PTHREAD_MUTEX_INITIALIZER;

void log_message(const char* message) {
    pthread_mutex_lock(&buffer_mutex);
    strncpy(ring_buffer.messages[ring_buffer.tail], message, MAX_LOG_LENGTH - 1);
    ring_buffer.messages[ring_buffer.tail][MAX_LOG_LENGTH - 1] = '\0';
    ring_buffer.tail = (ring_buffer.tail + 1) % RING_BUFFER_SIZE;
    if (ring_buffer.count < RING_BUFFER_SIZE) {
        ring_buffer.count++;
    } else {
        ring_buffer.head = (ring_buffer.head + 1) % RING_BUFFER_SIZE;
    }
    pthread_mutex_unlock(&buffer_mutex);
}

void dump_buffer_to_file(const char* filename) {
    FILE* file = fopen(filename, "w");
    if (file == NULL) {
        fprintf(stderr, "Error opening file %s\n", filename);
        return;
    }

    pthread_mutex_lock(&buffer_mutex);
    int i;
    for (i = 0; i < ring_buffer.count; i++) {
        int index = (ring_buffer.head + i) % RING_BUFFER_SIZE;
        fprintf(file, "%s\n", ring_buffer.messages[index]);
    }
    pthread_mutex_unlock(&buffer_mutex);

    fclose(file);
}

static int l_log(lua_State* L) {
    const char* message = luaL_checkstring(L, 1);
    log_message(message);
    return 0;
}

static int l_dump_log(lua_State* L) {
    dump_buffer_to_file("koreader.log");
    return 0;
}

void* lua_thread(void* arg) {
    lua_State* L = luaL_newstate();
    luaL_openlibs(L);

    lua_pushcfunction(L, l_log);
    lua_setglobal(L, "log");

    lua_pushcfunction(L, l_dump_log);
    lua_setglobal(L, "dump_log");

    if (luaL_dofile(L, "script.lua") != LUA_OK) {
        const char* error_msg = lua_tostring(L, -1);
        log_message("Lua VM died unexpectedly");
        log_message(error_msg);
        dump_buffer_to_file("crash.log");
    }

    lua_close(L);
    return NULL;
}

int main() {
    pthread_t thread;
    pthread_create(&thread, NULL, lua_thread, NULL);
    pthread_join(thread, NULL);
    return 0;
}

[pazos]

I have no real opinion one way or the other, though I do wonder if it might be harder to debug? Running luajit directly provides easy access to various helpful arguments.

I guess you're referring to arguments passed to the luajit binary and not those passed to reader.lua?. We already handle that, see
https://github.com/koreader/koreader-base/blob/master/osx_loader.c#L68-L81

If you have some example in mind I would like to know. We can parse arguments conditionally instead of forward them all to the state.

Android is not a priority for me (although one of my main goals to request this is to avoid ANRs by processing input on a different thread). I would start with the desktop/emulator branch and test on embedded linux first.

[Frenzie]

I mean just about every single thing on this page: https://luajit.org/running.html
But granted, practically speaking probably little more than JIT on/off and the profiler. (And for the profiler you generally have to stick it in the code around what you want to profile anyway, lest you get a lot of stuff you're not interested in.)

I think anything of (on-device) interest can also be done in the form of code. Doing it as a command-line argument is just a tad quicker.

[pazos]

I like how emacs bootstraps itself.

We could mimic that.

Something like koreader -q starts koreader without any userpatch
Something like koreader --debug-init starts a luajit interpreter that's able to run KO.

So, something like koreader --debug-init $(luajit arguments) reader.lua would start KOReader in "debug init mode"

[poire-z]

Not a fan of this idea and making koreader even more monoli(b)tic :)
In any case, I'd like that we still ship a luajit and the current behaviour/boostraping can be preserved.

I do patch to print stuff, I do log not-in-debug, I do have things that grep my crash.log (including past runs, without crash) to compare book opening time, track battery level changes...
I'm not convinced by the ring buffer idea and writing logs only on occasions.

I can understand Run the native input loop on a different thread, which may have some purpose only on Android.

Additionally, there is some current features that fork and keep running the current state and die soon (partial rendering, page thumbnails, coverbrowser book extracts, wikipedia downloads), and I have no idea about the complexities/risks/ugliness of doing that from a 2nd thread.

[pazos]

In any case, I'd like that we still ship a luajit and the current behaviour/boostraping can be preserved.

Do you mean the cli REPL?. Because a koreader binary would be 99% a luajit binary. It can interpret lua code. By default it will execute reader.lua.

I do patch to print stuff, I do log not-in-debug

No problem with that.

I do have things that grep my crash.log (including past runs, without crash) to compare book opening time, track battery level changes...

I see some problem with that but hopefully we can preserve current behaviour (dump everything to crash.log, as it arrives) opt-in for cases like yours.

It would be better as you can grep your crash.log even on a desktop, which currently don't have one ;)

Additionally, there is some current features that fork and keep running the current state and die soon (partial rendering, page thumbnails, coverbrowser book extracts, wikipedia downloads), and I have no idea about the complexities/risks/ugliness of doing that from a 2nd thread.

I'm not sure. In theory everything should be transparent if we run everything on the same OS thread and all native modules use lua_newthread() to spawn instances inherited from the state.

[poire-z]

Anyway, it feels more like a caprice than solving any real problem - adding complexity (threads) where we neved needed them and can keep things simple (fork).

[NiLuJe]

I have no strong feelings about any of the points that were raised, but I do find running the input loop in a dedicated thread highly desirable ;).