Window is frozen

[supsm17]

Hi there,

I recently came across an issue where my window would freeze after some time. I thought it would have something to do with the output widget, which I recently switched from Fl_Text_Display to Fl_Browser (I had not seen this issue before). When I commented out the line that adds lines to the browser, the issue does not appear.

Things that are broken:

Nothing updates, including button press/unpress, scroll bars, check boxes, and text input.

Things that are not broken:

Buttons still seem to work? When I pressed one of the buttons which triggers another window to open, the window still opened. However, the main window was still frozen. The new window is not frozen.

Redrawing the window does not fix the issue, however hiding and showing the window does. The issue also does not always appear, but does most of the time.

My setup is as follows:

The main thread outputs some lines then has Fl::run, then when a certain button is pressed, a new thread is created and detached. The new thread runs some other calculations but outputs sometimes by calling the output function. The output function uses Fl_Browser::add, and adds lines in a format like this:  @S14@C88@.text   . I suspect it has something to do with multithreading, as the issue does not happen in the main thread.

Any help is appreciated.

[Manolo]

To call Fl_Browser::add() from a worker thread, you have to call Fl::lock() before and Fl::unlock() after.

See an example of how it's done in demo program test/threads.

Also, make sure you follow guidelines given in :

https://fltk.gitlab.io/fltk/advanced.html

[imm]

Yes, as Manolo says, the crux is that the render context on WIN32 is basically single threaded and can only safely be accessed from the main thread of each process.

So to access rendering from a worker thread, you need to lock, or (better) use the FL::awake callback mechanism.

--
Ian
From my Fairphone FP3
  

[supsm17]

I have tried to use Fl::awake, however printing still doesn't really work from worker threads.

Here is what happens:

I can print successfully from main thread

When I try to print from worker thread, the first time it prints, but then the thread disappears?

The printing is done and appears on the screen, but the thread itself does not do anything else and does not appear in the call stack

My callback function looks like this:

void print(void* v)

{

ui->output->add(static_cast<char*>(v));

}

[supsm17]

As for lock and unlock, they kind of work? The window is very laggy and takes some time to update. After the thread finishes executing it appears that the window is completely frozen.

On Sunday, November 28, 2021 at 3:43:51 AM UTC-8 Ian MacArthur wrote:

[imm]

On Sun, 28 Nov 2021, 20:46 supsm17 wrote:

As for lock and unlock, they kind of work? The window is very laggy and takes some time to update. After the thread finishes executing it appears that the window is completely frozen.

Hm, OK, I suspect this comes under the heading of "threads are hard", basically.

I'm on my phone, so can't really code up an example just now.

It would be helpful if you could post a minimal, compilable, example of what you are doing that manifests the failure state, so that we can advise on how to proceed.

This can (and should) work fine - that it does not tells us that there's something awry in your code, so we need to see what you are doing, so we can figure out what.

--
Ian
From my Fairphone FP3
   

On Sunday, November 28, 2021 at 3:43:51 AM UTC-8 Ian MacArthur wrote:

On Sun, 28 Nov 2021, 10:39 Manolo wrote:

To call Fl_Browser::add() from a worker thread, you have to call Fl::lock() before and Fl::unlock() after.

See an example of how it's done in demo program test/threads.

Also, make sure you follow guidelines given in :

https://fltk.gitlab.io/fltk/advanced.html

Yes, as Manolo says, the crux is that the render context on WIN32 is basically single threaded and can only safely be accessed from the main thread of each process.

So to access rendering from a worker thread, you need to lock, or (better) use the FL::awake callback mechanism.

--
Ian
From my Fairphone FP3
  

--
You received this message because you are subscribed to the Google Groups "fltk.general" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkgeneral...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/fltkgeneral/930e721d-4742-41ba-9fb5-24bde3274b5en%40googlegroups.com.

[supsm17]

I found out the issue, it turns out I had a leftover Fl::wait() which caused the window to be laggy and thread to hang. However, I found that Fl::awake sometimes outputs garbage, and I'm not sure why. Fl::lock and unlock work perfectly fine.

This is the output I receive:

Here is the code:

#include <iostream>

#include <memory>

#include <string>

#include <thread>

#include "FL/Fl.H"

#include "FL/Fl_Double_Window.H"

#include "FL/Fl_Browser.H"

#include "FL/Fl_Check_Button.H"

void run(Fl_Button*, void*);

class UI

{

public:

Fl_Double_Window* window;

Fl_Browser* output;

UI() {

{ window = new Fl_Double_Window(660, 420, "window");

{ Fl_Button* o = new Fl_Button(445, 370, 190, 30, "Start");

o->labelfont(1);

o->labelsize(16);

o->callback((Fl_Callback*)run);

} // Fl_Button* o

{

output = new Fl_Browser(25, 40, 395, 275);

output->box(FL_BORDER_BOX);

output->textfont(4);

} // Fl_Browser* output

{ Fl_Check_Button* o = new Fl_Check_Button(185, 380, 56, 16, "name");

o->down_box(FL_DOWN_BOX);

o->value(1);

} // Fl_Check_Button* o

}

}

void show() {

window->show();

}

};

std::unique_ptr<UI> ui;

void awake(void* v)

{

ui->output->add(static_cast<char*>(v));

}

void output(std::string s)

{

std::cout << const_cast<char*>(("@S14@C92@." + s).c_str()) << std::endl;

Fl::awake(awake, const_cast<char*>(("@S14@C92@." + s).c_str()));

}

void thread()

{

output("a");

std::this_thread::sleep_for(std::chrono::seconds(1));

output("b");

int temp = 1;

for (int i = 1; i < 1000000000; i++)

{

temp += i;

}

output(std::to_string(temp));

}

void run(Fl_Button* o, void* v)

{

std::thread t(thread);

t.detach();

}

int main(int argc, char* argv[])

{

ui = std::make_unique<UI>();

Fl::get_system_colors();

Fl::lock();

ui->window->show();

for (int i = 0; i < 20; i++)

{

output(std::to_string(i));

}

Fl::run();

}

[Manolo]

Le lundi 29 novembre 2021 à 09:47:09 UTC+1, sups...@gmail.com a écrit :

void output(std::string s)

{

std::cout << const_cast<char*>(("@S14@C92@." + s).c_str()) << std::endl;

Fl::awake(awake, const_cast<char*>(("@S14@C92@." + s).c_str()));

}

 I believe the error is in that function, output(), in the call to Fl::awake().

The memory storage of the 2nd argument sent to Fl::awake is unstable, it can be reused at any time

after return from the Fl::awake() function.

Change that 2nd argument to a form where you control the lifetime of this char array, and make sure it's not

changed until it's been used by the main thread.

[Ian MacArthur]

On Monday, 29 November 2021 at 09:02:06 UTC Manolo wrote:

 I believe the error is in that function, output(), in the call to Fl::awake().

The memory storage of the 2nd argument sent to Fl::awake is unstable, it can be reused at any time

after return from the Fl::awake() function.

Change that 2nd argument to a form where you control the lifetime of this char array, and make sure it's not

changed until it's been used by the main thread.

 Yes - I agree with Manolo.

As I said earlier "threads are hard". 

The key point here is that the output() function will be called "now" in the context of the worker thread, and the passed string s will hold the expected value at that time.

However, the awake() function will be called "later" in the main context, at which point the string it is passed (via the void *v) may have expired and you get rubbish instead.

You need to ensure that any parameters passed to the awake() function have a lifetime sufficient for the purpose.

The asynchronous nature of threads can be challenging....

[Ian MacArthur]

Coffee break: Here's a worked example based on the OP code - though note that I had to "de-C++ify" it to make it work with the somewhat ancient compiler on this particular machine!

fltk-config --compile test.cxx

-------------------

#include <memory>

#include <string>

#include "FL/Fl.H"

#include "FL/Fl_Double_Window.H"

#include "FL/Fl_Browser.H"

#include "FL/Fl_Button.H"

#include "FL/Fl_Toggle_Button.H"

#ifdef _WIN32

# include <process.h>

typedef unsigned long Fl_Thread;

extern "C" {

    typedef void * (__cdecl Fl_Thread_Func) (void *);

}

static int fl_create_thread (Fl_Thread &t, Fl_Thread_Func *f, void *p)

{

    return t = (Fl_Thread)_beginthread ((void (__cdecl *) (void *))f, 0, p);

}

#define DELAY(XX) Sleep(XX)

#else

# include <pthread.h>

typedef pthread_t Fl_Thread;

extern "C" {

    typedef void * (Fl_Thread_Func) (void *);

}

static int fl_create_thread (Fl_Thread &t, Fl_Thread_Func *f, void *p)

{

    return pthread_create ((pthread_t *)&t, 0, f, p);

}

#define DELAY(XX) usleep(XX * 1000)

#endif

static Fl_Thread t = (Fl_Thread)0;

static volatile bool thread_alive = false;

static bool keep_alive = false;

static void run_once (Fl_Button *, void *);

static void run_many (Fl_Button *, void *);

class UI

{

public:

    Fl_Double_Window *window;

    Fl_Browser *output;

    UI()

    {

        {

            window = new Fl_Double_Window (660, 420, "Thread Test");

            window->begin();

            Fl_Button *bt = new Fl_Button (445, 370, 190, 30, "Run Once");

            bt->labelfont (1);

            bt->labelsize (16);

            bt->callback ((Fl_Callback *)run_once);

            bt = new Fl_Toggle_Button (445, 330, 190, 30, "Run Many");

            bt->labelfont (1);

            bt->labelsize (16);

            bt->callback ((Fl_Callback *)run_many);

            output = new Fl_Browser (25, 40, 395, 275);

            output->box (FL_BORDER_BOX);

            output->textfont (4);

        }

    }

};

static std::unique_ptr<UI> ui;

static volatile bool in_use = false;

static char text_buffer [128];

static void awake (void *)

{

    if (in_use)

    {

        ui->output->add (text_buffer);

        int ln = ui->output->size();

        ui->output->bottomline (ln);

        in_use = false;

    }

}

static void output (const char *pc)

{

    if (!in_use)

    {

        snprintf (text_buffer, 128, "@S14@C92@.%s", pc);

        in_use = true;

        puts (text_buffer);

        Fl::awake (awake, NULL);

    }

}

static void output (std::string s)

{

    output (s.c_str());

}

static void *thread (void *)

{

    while (thread_alive)

    {

        output ("a");

        DELAY (1000);

        output ("b");

        unsigned temp = 1;

        for (int i = 1; i < 1000000000; i++)

        {

            temp += i;

        }

        output (std::to_string (temp));

        if (!keep_alive)

        {

            thread_alive = false;

        }

        else

        {

            // busy wait for awake to flush the previous message

            while (in_use);

        }

    }

    // busy wait for awake to flush the message

    while (in_use);

    output ("Worker Thread Exit");

    t = (Fl_Thread)0;

    return NULL;

}

static void run_once (Fl_Button *, void *)

{

    if (!t)

    {

        thread_alive = true;

        fl_create_thread (t, thread, NULL);

    }

}

static void run_many (Fl_Button *, void *)

{

    if (!t)

    {

        thread_alive = true;

        keep_alive = true;

        fl_create_thread (t, thread, NULL);

    }

    else

    {

        keep_alive = false;

    }

}

int main (int argc, char *argv[])

{

    ui = std::make_unique<UI>();

    Fl::lock();

    ui->window->show (argc, argv);

    // Only the first of these strings will appear, since the

    // worker thread is

    for (int i = 0; i < 20; i++)

    {

        char buf [32];

        snprintf (buf, 32, "%d", i);

        ui->output->add (buf);

        int ln = ui->output->size();

        ui->output->bottomline (ln);

    }

    Fl::run();

    // Tell the worker to expire - if it is still alive!

    keep_alive = false;

    thread_alive = false;

    return 0;

}

// end of file

 

[supsm17]

Thanks for all the help. I usually don't use pointers frequently so I apologize for the dumb mistake.

[Ian MacArthur]

On Tuesday, 30 November 2021 at 07:37:01 UTC sups... wrote:

Thanks for all the help. I usually don't use pointers frequently so I apologize for the dumb mistake.

There was nothing "wrong" with your use of pointers, per se, rather the issue is the asynchronous nature of multi-thread code, and how that interacts with the lifetimes of the various variables passed between the threads.

The key thing is that the callback function passed to the awake() call is not executed at that point in time in the worker thread, but rather is executed "later" in the main thread - and we do not know when. So any variables that are created (in the worker thread) and passed to the callback function need to be created in such a way that they still exist (and have not changed their value) at the (unknown) future time at which the callback actually executes... It's tricky...

In your original example, some of the strings you were using were created in the worker thread, but the worker thread then expired (taking its variables with it...) so that by the time the callback function tried to use these variables later, they did not refer to anything any more, hence the garbage output and other misbehaviours you observed.

This is a common "pain-point" for asynchronous coding, pretty much everybody struggles with this.

The other common alternative is to use a lock / unlock approach - this allows the worker thread to perform (some) operations immediately "now", alleviating the need to preserve the lifetime of the variables, because they no longer need to be accessed asynchronously by another thread. The disadvantage of locks, however, is that they cause threads to block until all the threads involved can synchronize and thus also tend to serialize actions that could/should otherwise be asynchronous.

For things like rendering to the screen, where the process has only one render context, any thread that accesses that context via locks can then end up serialized with the render context, and in the worst cases you no longer have multi-threaded code at all, rather you have a pointlessly complicated single thread, in effect...

With modern multi-core CPUs, asynchronous threads *usually* make more effective use of the available computing resources - but the downside is the potential complexity of managing the lifetimes of shared data. There are many patterns that are intended to help with that...