glsl-optimizer icon indicating copy to clipboard operation
glsl-optimizer copied to clipboard

Thread safety issues (was: calling glslopt_initialize/glslopt_cleanup many times)

Open eodabash opened this issue 9 years ago • 5 comments

We build the glsl optimizer source directly into an ios app and (perhaps not the best practice) create a new glslopt_ctx for each shader we compile at runtime. Recently (possibly as of Xcode 6) we seem to be dying sometimes inside glslopt_cleanup with memory errors, or glslopt_optimize starts failing with a lot of error messages about seemingly correct code.

Before I do a lot of digging to try to narrow down a test case I wondering if creating multiple glslopt contexts per process/GL context/whatever is even safe to do, or if there is global state being stored somewhere.

eodabash avatar Dec 03 '14 07:12 eodabash

Or if it is safe to have multiple contexts, is it safe to call glslopt_optimize (w/different contexts) from multiple threads?

eodabash avatar Dec 03 '14 07:12 eodabash

Yeah looks like thread safety-ness issue. If you replace the main function in the tests project with this code things should fall apart pretty quickly (reproducible on Windows too).

int main (int argc, const char** argv)
{
    auto test = []() -> void
    {
        std::string inputPath = "C:/git/glsl-optimizer/tests/vertex/uniforms-arrays-inES3.txt";
        std::string input;
        bool result = ReadStringFromFile(inputPath.c_str(), input);

        if (!result) {
            printf("couldn't load input file\n");
            return;
        }

        glslopt_ctx* ctx = glslopt_initialize(kGlslTargetOpenGLES30);

        glslopt_shader* shader = glslopt_optimize(ctx, kGlslOptShaderVertex, input.c_str(), 0);
        bool optimizeOk = glslopt_get_status(shader);

        if (optimizeOk) {
            std::string textHir = glslopt_get_raw_output(shader);
            std::string textOpt = glslopt_get_output(shader);
        }

        else {
            std::string errorLog = glslopt_get_log(shader);
            printf("error: %s\n", errorLog.c_str());
        }

        glslopt_cleanup(ctx);
    };


    for (int i = 0 ; i < 100 ; ++i) {
        std::thread t1(test);
        std::thread t2(test);

        t1.join();
        t2.join();
    }   

    return 0;
}

eodabash avatar Dec 03 '14 18:12 eodabash

I can confirm this is true. I have the same problem when used with multiple threads. Is it using some kind of shared state or buffer inside? I believe the problem lies in glslopt_optimize function. My case is that i call this function from different threads for different shaders. If I skip the optimization everything works great. Any knowlege on this issue will be appreciated

volcoma avatar Dec 14 '16 01:12 volcoma

I think the issue is this stuff in builtin_functions.cpp

//@TODO: implement typedef int mtx_t; #define _MTX_INITIALIZER_NP 0 #define mtx_lock(name) #define mtx_unlock(name)

Plus these globals in glsl_types.cpp

hash_table *glsl_type::array_types = NULL; hash_table *glsl_type::record_types = NULL; hash_table *glsl_type::interface_types = NULL; void *glsl_type::mem_ctx = NULL;

If I supply an implementation for the first group and add similar synchronization around the second, then the test case I posted originally doesn't crash anymore.

eodabash avatar Dec 14 '16 05:12 eodabash

Indeed, it very well might be that there's some part that is not thread safe somewhere. Did not have time to dig in & find it though.

aras-p avatar Jan 11 '17 10:01 aras-p