Mir

Merge lp:~thomas-voss/mir/revert-process-group-leader-patch into lp:~mir-team/mir/trunk

Proposed by Thomas Voß
Status: Merged
Approved by: kevin gunn
Approved revision: no longer in the source branch.
Merged at revision: 898
Proposed branch: lp:~thomas-voss/mir/revert-process-group-leader-patch
Merge into: lp:~mir-team/mir/trunk
Diff against target: 30 lines (+0/-20)
1 file modified
src/server/graphics/gbm/linux_virtual_terminal.cpp (+0/-20)
To merge this branch: bzr merge lp:~thomas-voss/mir/revert-process-group-leader-patch
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
kevin gunn (community) Approve
Maarten Lankhorst (community) Disapprove
Review via email: mp+177645@code.launchpad.net

Commit message

Don't make Mir become a process group leader, to avoid CTRL-C killing the X server.

Description of the change

Don't make Mir become a process group leader, to avoid CTRL-C killing the X server.

To post a comment you must log in.
Revision history for this message
Robert Ancell (robert-ancell) wrote :

For the record, this is reverting this change: https://code.launchpad.net/~mlankhorst/mir/setsid/+merge/176676

Revision history for this message
Robert Ancell (robert-ancell) wrote :

This change was to fix the issue where the VT couldn't be acquired on startup, i.e.

"Failed to set the current VT mode: Input/output error (5)"

It's not clear to me how this change fixed that, but we should be aware that reverting this change might cause that problem again.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Maarten Lankhorst (mlankhorst) wrote :

See xorg-server/hw/xfree86/os-support/linux/lnx_init.c xf86OpenConsole and do the same for mir. Use tcgetattr/tcsetattr to set the console parameters to IGNBRK, this should fix xorg-server. You probably also need to set KDSKBMUTE to prevent characters from being visible afterwards.

review: Disapprove
Revision history for this message
kevin gunn (kgunn72) wrote :

for the moment - we are trying to update the system-compositor-testing ppa & this cntl+c issue is worse than the original bug that https://code.launchpad.net/~mlankhorst/mir/setsid/+merge/176676 fixed

top approving this, we can revisit this topic after we update the system-compositor-testing ppa

Revision history for this message
kevin gunn (kgunn72) wrote :

and an approve...

review: Approve
Revision history for this message
kevin gunn (kgunn72) wrote :

so we just verified through some additional testing that this change is only needed when the xmir stack fails to open the hw gpu driver & falls back to sw rendering for the x stack (not mir, but x)
So, the bug is still relevant but this change should be abandoned in favor of a solution which addresses both bugs

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
Approved revid is not set in launchpad. This is most likely a launchpad issue and re-approve should fix it. There is also a chance (although a very small one) this is a permission problem of the ps-jenkins bot.
http://jenkins.qa.ubuntu.com/job/mir-autolanding/506/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1490
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1375
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-autolanding/160
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-autolanding/160/artifact/work/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Revision history for this message
kevin gunn (kgunn72) wrote :

oops...hit save too soon...
these are the 2 relevant competing bugs...
https://bugs.launchpad.net/xmir/+bug/1206508 "Xorg crash on xmir punts user back out to greeter" which is the ctl+c issue on sw rendering - which this mp was intending to fix

and https://bugs.launchpad.net/mir/+bug/1195509 "System compositor fails to start - Failed to set the current VT mode: Input/output error (5) " which is a infrequent occuring bug...and results in fall back, which is fixed by https://code.launchpad.net/~mlankhorst/mir/setsid/+merge/176676

review: Disapprove
Revision history for this message
kevin gunn (kgunn72) wrote :

ok, after more testing - this was relevant to when x is 3D rendering, not tied to sw rendering as first thot
approving

still need to solve the conflict of desire to fix aforementioned bugs

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/server/graphics/gbm/linux_virtual_terminal.cpp'
2--- src/server/graphics/gbm/linux_virtual_terminal.cpp 2013-07-26 05:12:14 +0000
3+++ src/server/graphics/gbm/linux_virtual_terminal.cpp 2013-07-30 16:37:23 +0000
4@@ -186,26 +186,6 @@
5
6 std::string const active_vt_path{vt_path_stream.str()};
7
8- if (activate)
9- {
10- if (getpid() == getpgid(0) && setpgid(0, getpgid(getppid())) < 0)
11- {
12- BOOST_THROW_EXCEPTION(
13- boost::enable_error_info(
14- std::runtime_error("Failed to stop being a process group"))
15- << boost::errinfo_errno(errno));
16- }
17-
18- /* become process group leader */
19- if (setsid() < 0)
20- {
21- BOOST_THROW_EXCEPTION(
22- boost::enable_error_info(
23- std::runtime_error("Failed to become session leader"))
24- << boost::errinfo_errno(errno));
25- }
26- }
27-
28 auto vt_fd = fops->open(active_vt_path.c_str(), O_RDONLY | O_NDELAY);
29
30 if (vt_fd < 0)

Subscribers

People subscribed via source and target branches