Bug 4387 - Displays must not be shared across tet_fork
Summary: Displays must not be shared across tet_fork
Status: RESOLVED MOVED
Alias: None
Product: Xtests
Classification: Unclassified
Component: XTS (show other bugs)
Version: unspecified
Hardware: All All
: high normal
Assignee: Stuart Anderson
QA Contact: Xorg testing team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-09-07 10:44 UTC by Jamey Sharp
Modified: 2018-08-10 21:43 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch to ensure no test shares displays across tet_fork (3.43 KB, patch)
2005-09-07 10:46 UTC, Jamey Sharp
Details | Splinter Review
Don't share displays across tet_fork, and make event test more deterministic (3.75 KB, patch)
2005-09-07 19:28 UTC, Jamey Sharp
Details | Splinter Review
libxcb fix (1.44 KB, patch)
2010-08-15 21:50 UTC, Aaron Plattner
Details | Splinter Review

Description Jamey Sharp 2005-09-07 10:44:57 UTC
The XCloseDisplay purpose #6, XGrabDisplay, and XUngrabDisplay tests all open an
extra connection and then tet_fork off a new process to do stuff with that
connection. After forking these tests also manipulate the extra connection from
the parent.

Since the Display structure has a lot of state that has to be kept in sync with
the server, it isn't reasonable to expect Xlib to deal with having the same
Display used in two separate processes. Xlib/XCB has assertions that sanity
check the sequence number stream, and that stream is corrupted by this bug in
the test suite; the absence of these assertions in the reference implementation
of Xlib presumably explains why this bug hasn't been noticed before.

All three tests open these extra displays with the XTS function opendisplay,
which queues the Display to be closed at the end of the test. That doesn't work
since the XCloseDisplay call is made in the parent after the fork. The minimal
fix replaces the opendisplay() call with a call to XOpenDisplay(config.display),
which means that XCloseDisplay will not be automatically called. It can't be
called from the child in any of these cases, because that would make the child
hang until the server is ungrabbed; but of course it can't be called from the
parent either. For the XGrabServer and XUngrabServer tests I've chosen to let
the connection leak, because there's only one test purpose. For XCloseDisplay, I
get the file descriptor from the child's Display, and call close(2) on it in the
parent after the child exits. This leaks memory but ensures that all connections
to the server are closed at the end of the test, so the server can reset.

In addition, the XCloseDisplay test uses the "client2" connection in both parent
and child as part of the test procedure. The minimal fix for this opens a third
connection for use by the child, and allows the parent to continue using
client2. I discussed this part of the bug with Keith, who recommended this fix.

I have a patch that fixes all three tests.
Comment 1 Jamey Sharp 2005-09-07 10:46:22 UTC
Created attachment 3195 [details] [review]
Patch to ensure no test shares displays across tet_fork
Comment 2 Jamey Sharp 2005-09-07 19:28:35 UTC
Created attachment 3199 [details] [review]
Don't share displays across tet_fork, and make event test more deterministic

This is a minor revision to the previous patch. On Xfake, at least, the
XCloseDisplay test appears to succeed only rarely without a couple of extra
XSync calls. I'm not sure whether this bug affects the test when the previous
patch hasn't been applied, so I'm not opening it as a separate bug.
Comment 3 Egbert Eich 2006-09-21 09:33:46 UTC
Can this be closed?

For more references also see:
http://lists.x.org/archives/xorg-test/2005-December/000016.html
http://lists.x.org/archives/xorg-test/2005-December/000017.html
For discussion about the commit:
http://lists.x.org/archives/xorg-test/2006-February/000025.html
http://lists.x.org/archives/xorg-test/2006-February/000026.html
The patches for Xlib10/ungrbsrvr/ungrbsrvr.m and Xlib10/grbsrvr/grbsrvr.m
have been committed.

For the discussion of a different solution for Xlib3/clsdsply/ clsdsply.m 
by Geoff Clare see:
http://lists.x.org/archives/xorg-test/2006-April/000027.html
http://lists.x.org/archives/xorg-test/2006-April/000030.html
A slightly modified version of this has been committed to the tree.
Comment 4 Stew Benedict 2008-12-04 06:52:15 UTC
I think this should stay open. We (LSB) are seeing this test still fail, even with the 'Revision 1.3  2006-05-12 10:41:58  gwc' mods referenced in comment 3 on current xcb-based distributions.
Comment 5 Aaron Plattner 2010-08-15 21:50:04 UTC
Created attachment 37892 [details] [review]
libxcb fix

I posted a patch to the XCB mailing list that should fix this.  It's reviewed, but I don't have write access to libxcb so I can't merge it myself.

http://lists.freedesktop.org/archives/xcb/2010-August/006364.html
Comment 6 GitLab Migration User 2018-08-10 21:43:18 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xorg/test/xts/issues/8.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.