MirAL

Merge lp://staging/~alan-griffiths/miral/fix-1668651 into lp://staging/miral

  1. fix-1668651
  2. Merge into trunk
Proposed by Alan Griffiths
Status: Merged
Approved by: Gerry Boland
Approved revision: 536
Merged at revision: 525
Proposed branch: lp://staging/~alan-griffiths/miral/fix-1668651
Merge into: lp://staging/miral
Diff against target: 430 lines (+120/-41)
10 files modified
miral-shell/decoration_provider.cpp (+8/-8)
miral-shell/decoration_provider.h (+0/-2)
miral-shell/shell_main.cpp (+6/-2)
miral-shell/titlebar_window_manager.cpp (+3/-9)
miral-shell/titlebar_window_manager.h (+5/-1)
miral/CMakeLists.txt (+2/-1)
miral/internal_client.cpp (+61/-9)
miral/join_client_threads.h (+24/-0)
miral/runner.cpp (+11/-2)
miral/symbols.map (+0/-7)
To merge this branch: bzr merge lp://staging/~alan-griffiths/miral/fix-1668651
Reviewer Review Type Date Requested Status
Gerry Boland (community) Approve
Review via email: mp+318616@code.staging.launchpad.net

Commit message

miral-shell shutdown was a mess. Fixed.

Description of the change

miral-shell shutdown was a mess.

After fixing the segfault, the server closed down first leading to SIGHUP causing the server to shutdown with EXIT_FAILURE.

So changed the lifecycle callback.

Then operations on the decorations "client" were failing (because the connection had closed). So made it possible for the shell main() to join the client threads.

The server now closes cleanly.

Not sure this is the cleanest code, so any suggestions are welcome.

To post a comment you must log in.
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Note that on versions of Mir prior to 0.24 closing the server with a signal (e.g. SIGTERM) still leads to a segfault. That's because we lack mir::server::add_stop_callback() - or any way to hook into that mechanism for closing the server.

That affects 16.04LTS (Mir 0.21) but is "only" a crash on exit. I don't think we need to block. (But I'll think about possible solutions.)

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> That affects 16.04LTS (Mir 0.21) but is "only" a crash on exit. I don't think
> we need to block. (But I'll think about possible solutions.)

Found a solution

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I've considered maintaining an internal list of InternalClientRunners and calling join_client_thread() on all of them immediately *after* executing any client provided stop callbacks.

That would simplify the main() functions (miral-kiosk needs nothing, miral-shell just needs the hook to close the decorations provider).

It would also be possible to introduce a mechanism to notify internal clients of a close request by supplying overloads that provide a callback for this purpose.

But all that is providing convenience for simple usecases when the mechanisms provided here are needed to address the general case.

Revision history for this message
Gerry Boland (gerboland) wrote :

All looks ok!

review: Approve
Revision history for this message
Gerry Boland (gerboland) wrote :

Ok, this makes sense.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches