Not closing when try to exit in example "async"

[Escapingbug]

Description

In the example program "async", when I started the 10 seconds countdown, and try to exit during this time, it just hold there.

I did some debugging, seeing that is probably the tasks are waiting to exit. Maybe we should think about a method to interrupt or kill the thread nicely? Force to kill the thread may result in inconsistent states, but I think we really should provide a way to exit during task evaluating instead of just waiting for it to end.

Version / OS

• azul version: git version 2b9a791

• Operating system: Manjaro linux

• Windowing system (X11 or Wayland, Linux only): i3wm

Steps to Reproduce

Run the program, and starts the count down, and try exit.

Expected Behavior

Exit immediately

Actual Behavior

Still waiting for 10 seconds countdown.

Additional Information

[fschutt]

Yeah, I know about this, it actually stack overflows, then crashes for some reason. This is a problem that isn't really easy to solve, because the counter is on a background thread and I can't "kill" the background thread - on the other hand I also can't just exit and leave the background thread running.

or kill the thread nicely

That's kind of the problem, this is impossible, due to how threads work on an operating-system level. I can quit the application without cleaning up the threads, but then the problem is that the background thread is still running, so what should I do?

I know about this, added it to the cards here: https://github.com/maps4print/azul/projects/1#card-13479920 - but I am not sure how to actually solve this. As far as I know, you can only kill processes (both on Linux and on Windows), not individual threads.

[Escapingbug]

The stackoverflow problem is caused by how errors are handled. Currently app::RuntimeError's fmt::Debug implementation has a problem, it uses write!(f, "{:?}", self), which will call itself once and once again.

Since I'm not sure how you want this error be handled, I couldn't fix this problem.

Actually you are right, I was thinking about this when I found this error. We can force exit the thread by drop(maybe? I tried it, and it did quite), but the state will be inconsistent. Maybe we really should let user add some "guardian" to task, so when it is forced to quit, one can do some recovery job(fix some inconsistent state?), or None when no need for that.

I'm also curious about why that Arc::try_unwrap returns error, since I'm not very familiar with the whole structure of the code.

BTW, this is a good repo, maybe it could change the future how rust people write gui applications. Looking forward to helping(although I'm no expert in gui or client-side application area).

[LordZane]

If you want to "kill" a background thread, create a channel, have the main thread hold an Option, and have the background thread hold the receiver. Each loop iteration of the background thread, it trys receiver.try_recv(), and if that fails due to connection dropped, stops the loop, ending the thread. When you want it to stop, simply replace the Some(Sender) with None on the main thread

[fschutt]

@LordZane Yes, but you have to understand that the framework has no control over the thread. The task is written by the application programmer, I can't "pause" the execution of a thread from azul. Only the person who uses azul (i.e. you) can control what happens inside the thread. So you would have to create the channel / write the loop. And that's already possible, the question is what happens when a thread isn't programmed in that way and the application exits all windows - should the data model stay alive or not?

[LordZane]

I'm not sure I understand the issue. Once the main thread ends (all window are closed), the OS should cleanup the background threads, no? No manual cleanup should be necessary

[Escapingbug]

I think I can explain a little bit about this.

The tasks are defined by users, one thread for each task will be run when it starts. But when main thread is trying to exit, all tasks, which are defined by users, not guaranteed to exit right away. Currently, there's even no information transferred to tasks, they don't know that the main thread is exiting, the main thread is only doing threads joining, waiting for their completions.

So, the main thread actually is not ended unless all tasks are completed, using a join.

What we need is to decide what should happen once a user tries to exit before tasks' completion.

Actually, I think it is fine to just end main thread without joining, although this may cause problem. This should be a problem for users to solve.

But how am I getting ArcUnlockError when force exiting?

[fschutt]

Yeah I'll investigate the ArcUnlockError tomorrow, that shouldn't happen.

[Escapingbug]

Ok, I have already done that. It is because that tasks lifetime is not ended when run_inner returns. It actually ends when run is returning. That is to say, when you do that try_unwrap, it is not dropped.

[Escapingbug]

I have sent a pull request to fix ArcUnlockError problem. Now we should decide if we should force to exit(just exit main thread, and let OS do sub-threads collection).

[fschutt]

Right, so what I think would be the right decision to solve this:

1. In the AppConfig I'll make a field if the application should exit if there are still non-main threads running after all windows are closed (via std::process::exit) - and if yes, with what exit code. Exiting destroys the entire process, and therefore the OS can clean up the threads immediately.
2. Panicking threads / tasks currently crash the entire app. AppConfig should have another field to set if (and how) azul should recover from crashed / poisoned tasks.
3. The default value (None) should let the tasks running until they finish. This is likely an application error, etc. but it's not the job of the framework to solve that, the application programmer has to solve it gracefully. The Arc<()> that gets passed into the task can be used to check if the main thread has exited. This means that tasks can run longer than the main thread (and an application can still be "running" even though all windows have been closed).
4. Update the async demo to demonstrate this new behaviour

I.e. something like this:

struct AppConfig { 
     // default: None - don't exit, rather let the threads run
     // if it's Some(x), then `std::process::exit(x)` - so:
     // Some(0) will call exit(0), Some(1) will call exit(1), etc.
    should_exit_after_windows_closed: Option<ExitCode>,
}

[LordZane]

1. Maybe better on a per-task basis? Do exit codes matter? Up to you
2. Is this necessary? Rust shouldn't crash after all, if there's an error, it should report it as a Result. Again, up to you

[Escapingbug]

1. Maybe better on a per-task basis? Do exit codes matter? Up to you

2. Is this necessary? Rust shouldn't crash after all, if there's an error, it should report it as a Result. Again, up to you

Agreed.

One task may be slow and not critical while another may be fast but critical. So we still want to wait until critical one to finish but leaving the not critical ones exit immediately. And I also think there should be a way for user to change how we react on windows when a closing event is received. Like, one should be able to "hold up" windows, and prompt some "waiting till _ to complete" message till waiting for background tasks.

And for the thread panicking stuff, it should just panic the whole application, because it represents an error instead of simple exception. That means, critical bugs that prevent the whole application to continue. For example, OOM. Then we really should not recover from these, or else, result should be returned.

[fschutt]

Well, these two problems have been fixed now, it doesn't crash anymore, it just closes the window, but the thread is still active. I do think that this would be an acceptable behaviour, because you wouldn't expect the framework to interfere with user code. Maybe there is indeed a task that should run longer than any window is open. So right now, it just closes the window immediately, but doesn't end the process, since the background thread is still running.

I am closing this for now, since the original issue (that the async example crashes / panics) is fixed. Feel free to reopen if you disagree with that choice.