glx X11 detection fails because it assumes pipefds are never 0

RESOLVED FIXED in Firefox 57

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: sebastian, Assigned: sebastian)

Tracking

52 Branch
mozilla57
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(2 attachments)

Assignee

Description

2 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.9) Gecko/20100101 Goanna/3.2 Firefox/45.9 PaleMoon/27.4.2
Build ID: 20170822051534

Steps to reproduce:

i'm using 52.2, but this is probably there since version 12.

over at enlightenment we have a strange issue: if firefox is started via enlightenment, glx X11 detection fails because strace shows the reply from /source/toolkit/xre/glxtest.cpp is correctly written, but never read. if started via a terminal - terminolgy - the detection works. bug report: https://phab.enlightenment.org/T5606

after reading the source code, i noticed in source/widget/GfxInfoX11.cpp:59 glxtest_pipe is tested against 0 to determine if GfxInfo::GetData() was already called. however glxtest_pipe is filled from pipefd[0] from a pipe(pipefd) call which can be 0. the solution should be simple, however i havn't tested it, initialise glxtest_pipe with -1 and check against -1 in  /widget/GfxInfoX11.cpp


Actual results:

glx detection fails.


Expected results:

glx detection should work.

Updated

2 years ago
Component: Untriaged → Widget: Gtk
OS: Unspecified → Linux
Product: Firefox → Core
Hardware: Unspecified → x86_64
Comment on attachment 8909275 [details] [diff] [review]
glxtest_pipe.patch

Thank you for the analysis and fix.

I assume most parent processes will open /dev/null fd 0 if they don't want to
pass on their own stdin.

Processes often test whether fd 0 is a terminal, and that test is likely to
get confused if fd 0 comes from a random source.
Attachment #8909275 - Flags: review+
Comment hidden (mozreview-request)

Comment 3

2 years ago
mozreview-review
Comment on attachment 8909544 [details]
bug 1400839 use -1 instead of 0 to indicate absent glxtest_pipe fd

https://reviewboard.mozilla.org/r/181018/#review186240
Attachment #8909544 - Flags: review?(karlt) → review+
Assignee: nobody → sebastian
Status: UNCONFIRMED → ASSIGNED
Component: Widget: Gtk → Graphics
Ever confirmed: true

Comment 4

2 years ago
Pushed by ktomlinson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fd4481cbbd29
use -1 instead of 0 to indicate absent glxtest_pipe fd r=karlt
https://hg.mozilla.org/mozilla-central/rev/fd4481cbbd29
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee

Comment 6

2 years ago
i can confirm grpahic detection now works in firefox nightly under enlightenment.

will this fix be backported to 55/56/esr?
Typically only recent regressions, frequent crashes, or security bugs are backported to more stable branches.
AIUI this is a long-standing issue.  It's also very late in the 56 beta cycle for this kind of fix.

https://wiki.mozilla.org/Release_Management/Uplift_rules#Beta_Uplift_.28approval-mozilla-beta.29
https://wiki.mozilla.org/RapidRelease/Calendar
You need to log in before you can comment on or make changes to this bug.