GLVis / glvis

Lightweight OpenGL tool for accurate and flexible finite element visualization
http://glvis.org
BSD 3-Clause "New" or "Revised" License
249 stars 51 forks source link

[BUG] Fixed init of the main thread #298

Closed najlkin closed 1 month ago

najlkin commented 1 month ago

This PR fixes initialization of the main thread object, which was sometimes happening in a non-main thread first and as it calls SDL_Init(), it was causing crashes on Mac (including CI runners ❗ ).

v-dobrev commented 1 month ago

I have not noticed this issue on Mac. How can I try to reproduce it? Also, which CI jobs failed due to this issue?

najlkin commented 1 month ago

It is random, depends what thread (main or worker) is first, but have a look at one of the many attempts in #296 , but it did not appear there first, I have seen that before, but just re-running the job saved the day usually.

tzanio commented 1 month ago

I also haven't noticed this, does it happens with reading from files only?

justinlaughlin commented 1 month ago

Here is an example of this failure on glvis-4.3-dev

https://github.com/GLVis/glvis/actions/runs/10083308934/job/27879591928?pr=284#step:23:602

najlkin commented 1 month ago

I believe it can happen with any input, we just reproduced that with @v-dobrev on his Mac with an extra sleep(). The thing is that SDLMainLoop(), which calls this GetMainThread() is called just after the thread (or Session object which does it internally) is created.

v-dobrev commented 1 month ago

@tzanio, here's a diff on master that allowed me to reproduce the issue (when loading a file with -saved):

diff --git a/glvis.cpp b/glvis.cpp
index 67d755e..9c255e9 100644
--- a/glvis.cpp
+++ b/glvis.cpp
@@ -1498,6 +1498,7 @@ int main (int argc, char *argv[])
          return 1;
       }

+      std::this_thread::sleep_for(std::chrono::milliseconds{100});
       SDLMainLoop();
       return 0;
    }
tzanio commented 1 month ago

Okay, thanks guys. I trust you