Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: corocbk.lua: terminate MainLoop on exit #303

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sfesenko
Copy link

Test (whole process) fails for luajit2.1 without correct termination of main loop

 Test fails for luajit without correct termination of main loop
@psychon
Copy link
Collaborator

psychon commented Nov 28, 2022

How is this code even reached? By the time this call to g_main_loop_quit() is executed, g_main_loop_run() must already have terminated, so... the main loop isn't running?

Put differently: Is this really fixing a bug in the test or is this just modifying the test to mask "the real problem"?

@sfesenko
Copy link
Author

IMO, error doesn't terminate main loop, so after pcall program has illegal state (exit from main loop without terminating it).

@psychon
Copy link
Collaborator

psychon commented Nov 29, 2022

So, in other news, the whole test is invalid and needs to be deleted? (Or better: Replaced with something else that still tests whatever this test is supposed to test, but without being invalid)

What exactly does "doesn't terminate main loop" mean? Obviously g_main_loop_run() is no longer running...?

@sfesenko
Copy link
Author

sfesenko commented Nov 30, 2022

What exactly does "doesn't terminate main loop" mean? Obviously g_main_loop_run() is no longer running...?

I guess, without g_main_loop_quit() call main loop is still active, so whole program enters abnormal state (since it exit main loop without g_main_loop_quit() call)

So, in other news, the whole test is invalid and needs to be deleted?

All tests are executed in the same context, so each test should be responsible for cleaning up allocated resources.
And, main loop must be terminated by g_main_loop_quit() before exit corocbk.rethrow()

To sum up:

  • If error should implicitly clean up all allocated resources (i.e. do g_main_loop_quit() call), then there is "the real problem", since this doesn't work.
  • Otherwise, g_main_loop_quit() just should be called explicitly, so main loop will be gracefully terminated and wont affect execution. (May be using xpcall with error handler that call g_main_loop_quit() would be more descriptive)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants