Tackle mcode_alloc issues once and for all

Makefile

@@ -77,10 +77,15 @@ update:
 	@echo "using sdk in path $(ANDROID_SDK_FULLPATH)"
 	@echo "using ndk in path $(ANDROID_NDK_FULLPATH)"
 
+build-luajit-debug:
+	@echo "Building LuaJIT for $(ANDROID_FULL_ARCH) (debug)"
+	cd jni/luajit && \
+		./mk-luajit.sh "$(ANDROID_FULL_ARCH)" "debug"
+
 build-luajit:
 	@echo "Building LuaJIT for $(ANDROID_FULL_ARCH)"
 	cd jni/luajit && \
-		./mk-luajit.sh $(ANDROID_FULL_ARCH)
+		./mk-luajit.sh "$(ANDROID_FULL_ARCH)"
 
 prepare: update
 	@echo "Building LuaJIT for all supported ABIs"
@@ -90,7 +95,7 @@ prepare: update
 		./mk-luajit.sh clean && \
 		./mk-luajit.sh armeabi-v7a
 
-debug: update build-luajit
+debug: update build-luajit-debug
 	@echo "Building $(APPNAME) debug APK: Version $(NAME), release $(VERSION), flavor $(FLAVOR)"
 	./gradlew -PversName=$(NAME) -PversCode=$(VERSION) -PprojectName=$(APPNAME) app:$(BUILD_TASK)Debug
 	mkdir -p bin/

app/src/main/AndroidManifest.xml

@@ -40,7 +40,7 @@
             android:screenOrientation="nosensor"
             android:launchMode="singleInstance"
             android:configChanges="colorMode|density|fontScale|keyboard|keyboardHidden|layoutDirection|locale|mcc|mnc|navigation|orientation|screenLayout|screenSize|smallestScreenSize|touchscreen|uiMode" >
-            <meta-data android:name="android.app.lib_name" android:value="luajit" />
+            <meta-data android:name="android.app.lib_name" android:value="luajit-launcher" />
             <intent-filter>
                 <action android:name="android.intent.action.MAIN" />
                 <category android:name="android.intent.category.LAUNCHER" />

assets/android.lua

@@ -7,19 +7,22 @@ Java Native Interface (JNI) wrapper.
 -- Attempt to grab the full maxmcode region in one go on startup,
 -- to avoid mcode_alloc failures later on at runtime...
 -- c.f., https://www.freelists.org/post/luajit/Android-performance-drop-moving-from-LuaJIT201-LuaJIT202
--- The other workaround mentioned earlier in that thread would require moving to a shared LuaJIT build,
--- and move the mmap hackery in jni/android-main.c before a dlopen of the luaJIT lib...
--- Without it, the most reliable results we can get are with a *single* 64K mcode block:
--- trying that with a single 512K block (as that's the default maxmcode) doesn't yield great results...
+-- For optimal behavior, this relies on a few LuaJIT hacks:
+--   * Ensuring a flush doesn't unmap the mcarea, but only clears it,
+--     because when the mcarea is filled, LuaJIT flushes it,
+--     and the Lua blitter can happily require more than 256K to flip a CRe page,
+--     so flushes are common.
+--   * Reserving a 1MB area inside LuaJIT's address space via a global array
+--   * Making the first mcode_alloc use the address of this global via MAP_FIXED
+-- c.f., koreader-luajit-mcode-reserve-hack.patch
 -- Upstream issue: https://github.com/LuaJIT/LuaJIT/issues/285
 
--- Hope that forcing the allocation of a 64K segment in two blocks *right now* will succeed...
-jit.opt.start("sizemcode=32", "maxmcode=64")
+-- Given that, force the allocation of a single 512K segment *right now*.
+-- (LuaJIT defaults are 32, 512 on 32-bit platforms, and 64, 512 otherwise).
+jit.opt.start("sizemcode=512", "maxmcode=512")
+-- This ensures a trace is generated, which requires an mcarea alloc ;).
 for _ = 1, 100 do end
 
--- Disable the JIT for now, we'll enable it again when actually starting KOReader.
-jit.off(true, true)
-
 local ffi = require("ffi")
 
 ffi.cdef[[
@@ -1137,16 +1140,17 @@ void AConfiguration_getCountry(AConfiguration* config, char* outCountry);
 ]]
 
 -- JNI Interfacing
+local C = ffi.C
 
 local JNI = {}
 
 function JNI:context(jvm, runnable)
     self.jvm = jvm
 
     local env = ffi.new("JNIEnv*[1]")
-    self.jvm[0].GetEnv(self.jvm, ffi.cast("void**", env), ffi.C.JNI_VERSION_1_6)
+    self.jvm[0].GetEnv(self.jvm, ffi.cast("void**", env), C.JNI_VERSION_1_6)
 
-    assert(self.jvm[0].AttachCurrentThread(self.jvm, env, nil) ~= ffi.C.JNI_ERR,
+    assert(self.jvm[0].AttachCurrentThread(self.jvm, env, nil) ~= C.JNI_ERR,
         "cannot attach JVM to current thread")
 
     self.env = env[0]
@@ -1191,15 +1195,15 @@ function JNI:callBooleanMethod(object, method, signature, ...)
     local clazz = self.env[0].GetObjectClass(self.env, object)
     local methodID = self.env[0].GetMethodID(self.env, clazz, method, signature)
     self.env[0].DeleteLocalRef(self.env, clazz)
-    return self.env[0].CallBooleanMethod(self.env, object, methodID, ...) == ffi.C.JNI_TRUE
+    return self.env[0].CallBooleanMethod(self.env, object, methodID, ...) == C.JNI_TRUE
 end
 
 function JNI:callStaticBooleanMethod(class, method, signature, ...)
     local clazz = self.env[0].FindClass(self.env, class)
     local methodID = self.env[0].GetStaticMethodID(self.env, clazz, method, signature)
     local res = self.env[0].CallStaticBooleanMethod(self.env, clazz, methodID, ...)
     self.env[0].DeleteLocalRef(self.env, clazz)
-    return res == ffi.C.JNI_TRUE
+    return res == C.JNI_TRUE
 end
 
 function JNI:callObjectMethod(object, method, signature, ...)
@@ -1249,38 +1253,46 @@ end
 
 -- Android specific
 
--- Some Android roms won't load libandroid.so to the global namespace thus
--- we load it by ourselves.
+-- We need to load libandroid, liblog, and the app glue: they're no longer in the global namespace
+-- as we're now running under a plain LuaJIT.
+-- NOTE: We haven't overloaded fii.load yet
+--       (and we can't, as our custom implementation depends on libandroid and liblog for logging ^^),
+--       so, we hope that the fact we've kept linking libluajit-launcher against libandroid and liblog will be enough
+--       to satisfy old and broken platforms where dlopen is extra finicky...
 local android_lib_ok, android_lib = pcall(ffi.load, "libandroid.so")
+local android_log_ok, android_log = pcall(ffi.load, "liblog.so")
+local android_glue_ok, android_glue = pcall(ffi.load, "libluajit-launcher.so")
 local android = {
     app = nil,
     jni = JNI,
     log_name = "luajit-launcher",
-    lib = android_lib_ok and android_lib or ffi.C,
+    lib = android_lib_ok and android_lib or C,
+    log = android_log_ok and android_log or C,
+    glue = android_glue_ok and android_glue or C,
 }
 
 function android.LOG(level, message)
-    ffi.C.__android_log_print(level, android.log_name, "%s", message)
+    android.log.__android_log_print(level, android.log_name, "%s", message)
 end
 
 function android.LOGVV(tag, message)
-    ffi.C.__android_log_print(ffi.C.ANDROID_LOG_VERBOSE, tag, "%s", message)
+    android.log.__android_log_print(C.ANDROID_LOG_VERBOSE, tag, "%s", message)
 end
 
 function android.LOGV(message)
-    android.LOG(ffi.C.ANDROID_LOG_VERBOSE, message)
+    android.LOG(C.ANDROID_LOG_VERBOSE, message)
 end
 function android.LOGD(message)
-    android.LOG(ffi.C.ANDROID_LOG_DEBUG, message)
+    android.LOG(C.ANDROID_LOG_DEBUG, message)
 end
 function android.LOGI(message)
-    android.LOG(ffi.C.ANDROID_LOG_INFO, message)
+    android.LOG(C.ANDROID_LOG_INFO, message)
 end
 function android.LOGW(message)
-    android.LOG(ffi.C.ANDROID_LOG_WARN, message)
+    android.LOG(C.ANDROID_LOG_WARN, message)
 end
 function android.LOGE(message)
-    android.LOG(ffi.C.ANDROID_LOG_ERROR, message)
+    android.LOG(C.ANDROID_LOG_ERROR, message)
 end
 
 --[[--
@@ -1296,7 +1308,7 @@ function android.asset_loader(modulename)
     local filename = string.gsub("?.lua", "%?", modulepath)
     local asset = android.lib.AAssetManager_open(
         android.app.activity.assetManager,
-        filename, ffi.C.AASSET_MODE_BUFFER)
+        filename, C.AASSET_MODE_BUFFER)
     --android.LOGI(string.format("trying to open asset %s: %s", filename, tostring(asset)))
     if asset ~= nil then
         -- read asset:
@@ -1774,8 +1786,8 @@ local function run(android_app_state)
             end)
         end,
         set = function(new_orientation)
-            if new_orientation >= ffi.C.ASCREEN_ORIENTATION_UNSPECIFIED and
-                new_orientation <= ffi.C.ASCREEN_ORIENTATION_FULL_SENSOR then
+            if new_orientation >= C.ASCREEN_ORIENTATION_UNSPECIFIED and
+                new_orientation <= C.ASCREEN_ORIENTATION_FULL_SENSOR then
                 JNI:context(android.app.activity.vm, function(jni)
                     jni:callVoidMethod(
                         android.app.activity.clazz,

assets/dl.lua

@@ -1,73 +1,95 @@
 --[[
 A LuaJIT FFI based version of dlopen() which loads dependencies
 first (for implementations of dlopen() lacking that feature, like
-on Android)
+on Android before API 23,
+c.f., https://android.googlesource.com/platform/bionic/+/refs/heads/master/android-changes-for-ndk-developers.md)
 
 This is heavily inspired by the lo_dlopen() implementation from
 LibreOffice (see
-http://cgit.freedesktop.org/libreoffice/core/tree/sal/android/lo-bootstrap.c)
+https://cgit.freedesktop.org/libreoffice/core/tree/sal/android/lo-bootstrap.c?id=963c98a65e4eddf179e170ff0bb30e4bfafc6b16)
 and as such:
 
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
 --]]
 
--- Disable the JIT in this module, in an attempt to avoid weird issues when loading libraries
+-- Disable the JIT in this module, to avoid weird and mysterious issues with dlopen (here and in ffi.load),
+-- as well as the nested loops in dl.dlopen mysteriously breaking early.
 jit.off(true, true)
 
 local ffi = require("ffi")
 local A = require("android")
 local Elf = require("elf")
 local log = "dlopen"
 
-ffi.cdef[[
-void *dlopen(const char *filename, int flag);
-char *dlerror(void);
-const static int RTLD_LOCAL = 0;
-const static int RTLD_GLOBAL = 0x00100;
-]]
+local C = ffi.C
+
+-- There's a bit of heinous hackery going on on 32-bit ABIs with RTLD_NOW & RTLD_GLOBAL...
+-- c.f., https://android.googlesource.com/platform/bionic/+/refs/heads/master/libc/include/dlfcn.h
+if jit.arch:sub(-2) == "64" then
+    ffi.cdef[[
+    void *dlopen(const char *filename, int flag);
+    char *dlerror(void);
+    const static int RTLD_LOCAL    = 0;
+    const static int RTLD_LAZY     = 0x00001;
+    const static int RTLD_NOW      = 0x00002;
+    const static int RTLD_NOLOAD   = 0x00004;
+    const static int RTLD_GLOBAL   = 0x00100;
+    const static int RTLD_NODELETE = 0x01000;
+    ]]
+else
+    ffi.cdef[[
+    void *dlopen(const char *filename, int flag);
+    char *dlerror(void);
+    const static int RTLD_LOCAL    = 0;
+    const static int RTLD_LAZY     = 0x00001;
+    const static int RTLD_NOW      = 0x00000;
+    const static int RTLD_NOLOAD   = 0x00004;
+    const static int RTLD_GLOBAL   = 0x00002;
+    const static int RTLD_NODELETE = 0x01000;
+    ]]
+end
 
 local dl = {
     -- set this to search in certain directories
     library_path = '/lib/?;/usr/lib/?;/usr/local/lib/?',
-
-    loaded_libraries = {}
 }
 
-local function sys_dlopen(library)
-    A.LOGVV(log, string.format("sys_dlopen - loading library %s", library))
-    local p = ffi.C.dlopen(library, ffi.C.RTLD_LOCAL)
+local function sys_dlopen(library, global, padding)
+    A.LOGVV(log, string.format("%"..padding.."ssys_dlopen - loading library %s (in %s namespace)", "", library, global and "global" or "local"))
+    local p = C.dlopen(library, bit.bor(C.RTLD_NOW, global and C.RTLD_GLOBAL or C.RTLD_LOCAL))
     if p == nil then
-        local err_msg = ffi.C.dlerror()
+        local err_msg = C.dlerror()
         if err_msg ~= nil then
             error("error opening "..library..": "..ffi.string(err_msg))
         end
+    else
+        C.dlerror()
     end
     return p
 end
 
 --[[
-open_func will be used to load the library (but not its dependencies!)
+load_func will be used to load the library (but not its dependencies!)
 if not given, the system's dlopen() will be used
 
 if the library name is an absolute path (starting with "/"), then
 the library_path will not be used
 --]]
-function dl.dlopen(library, load_func)
-    -- check if we already opened it:
-    if dl.loaded_libraries[library] then return dl.loaded_libraries[library] end
-
+function dl.dlopen(library, load_func, depth)
     load_func = load_func or sys_dlopen
+    depth = depth or 0
+    local padding = depth * 4
 
     for pspec in string.gmatch(
-            library:sub(1,1) == "/" and "" or dl.library_path,
+            library:sub(1, 1) == "/" and "" or dl.library_path,
             "([^;:]+)") do
 
         local lname, matches = string.gsub(pspec, "%?", library)
         if matches == 0 then
-            -- if pathspec does not contain a '?', we do append
-            -- the library name to the pathspec
+            -- if pathspec does not contain a '?',
+            -- we append the library name to the pathspec
             lname = lname .. '/' .. library
         end
 
@@ -77,29 +99,44 @@ function dl.dlopen(library, load_func)
             ok, lib = pcall(Elf.open, lname)
         end
         if ok then
-            A.LOGVV(log, string.format("dl.dlopen - library lname detected %s", lname))
+            A.LOGVV(log, string.format("%"..padding.."sdl.dlopen - %s => %s", "", library, lname))
+            -- Skip loading system libraries (unless we explicitly asked for one, in which case, that's on the caller ;)).
+            -- c.f., https://github.com/koreader/android-luajit-launcher/pull/69
+            -- Note that, technically, for Android >= 6.0, the list of safe system libraries is:
+            -- libandroid, libc, libcamera2ndk, libdl, libGLES, libjnigraphics,
+            -- liblog, libm, libmediandk, libOpenMAXAL, libOpenSLES, libstdc++,
+            -- libvulkan, and libz
+            -- Our current code should *never* hit any non-whitelisted system libs, so, this is basically overkill ;).
+            if depth > 0 and (pspec == "/system/lib" or library == "libdl.so") then
+                -- depth > 0 to allow explicitly loading a system lib
+                -- (because this might have genuine use cases, as some early API levels do not put DT_NEEDED libraries into the global namespace)
+                -- pspec to reject system libs
+                -- secondary check on libdl, because apparently there are old ROMs out there where it isn't in /sytem/lib ?!
+                A.LOGVV(log, string.format("%"..padding.."sdl.dlopen - skipping %s (system lib)", "", lname))
+                -- We won't load it, so, we don't even need to look at its deps.
+                lib:close()
+                return nil
+            end
+
+            depth = depth + 1
+            padding = depth * 4
             -- we found a library, now load its requirements
             -- we do _not_ pass the load_func to the cascaded
             -- calls, so those will always use sys_dlopen()
-            for _, needed in pairs(lib:dlneeds()) do
-                if needed == "libluajit.so" then
-                    --[[
-                    -- load the luajit-launcher libluajit with sys_dlopen
-                    load_func("libluajit.so")
-                    --]]
-                    -- Do *NOT* load libluajit if it's needed by something: we're already linked against a static copy!
-                    A.LOGVV(log, string.format("         dl.dlopen - skipping needed %s for %s", needed, lname))
-                elseif needed ~= "libdl.so" and pspec ~= "/system/lib" then
-                    -- For Android >= 6.0, the list of safe system libraries is:
-                    -- libandroid, libc, libcamera2ndk, libdl, libGLES, libjnigraphics,
-                    -- liblog, libm, libmediandk, libOpenMAXAL, libOpenSLES, libstdc++,
-                    -- libvulkan, and libz
-                    -- However, we have our own dl implementation and don't need the rest.
-                    A.LOGVV(log, string.format("         dl.dlopen - opening needed %s for %s", needed, lname))
-                    dl.dlopen(needed)
-                end
+            local lib_needs = lib:dlneeds()
+            lib:close()
+            for i, needed in ipairs(lib_needs) do
+                A.LOGVV(log, string.format("%"..padding.."sdl.dlopen - needed => %s (%d of %d) <= %s", "", needed, i, #lib_needs, lname))
+                dl.dlopen(needed, sys_dlopen, depth)
+            end
+            depth = depth - 1
+            padding = depth * 4
+            if load_func == sys_dlopen then
+                return sys_dlopen(lname, false, padding)
+            else
+                A.LOGVV(log, string.format("%"..padding.."sdl.dlopen - load_func -> %s", "", lname))
+                return load_func(lname)
             end
-            return load_func(lname)
         end
     end