Bug 1478 - Selection.c damages user error handler function
Summary: Selection.c damages user error handler function
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Lib/Xt (show other bugs)
Version: 6.8.1
Hardware: All All
: highest critical
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords: patch
: 1479 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-09-27 03:53 UTC by Michael
Modified: 2010-09-23 18:16 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
src/Selection.c (64.40 KB, text/plain)
2008-05-22 20:21 UTC, Ryan Hajdaj
no flags Details
Diff of HandleSelectionEvents (base is libXt-1.0.4) (4.77 KB, text/html)
2008-05-23 07:05 UTC, Ryan Hajdaj
no flags Details

Description Michael 2004-09-27 03:53:53 UTC
It looks like under certain conditions a user error handler routine set by 
XSetErrorHandler can be effectively canceled from inside of Xt/Selection.c.

The place to suspect is the loop in the SelectionRequest case of 
HandleSelectionEvents function. Under certain conditions there can be several 
two calls to EndProtectedSection after single StartProtectedSection. Since 
EndProtectedSection zeroes the global oldErrorHandler variable, the second 
call to EndProtectedSection restores the default error procedure instead of 
user's one.

Probably there exist more than one problematic code fragments.
(Checked in 6.6 and 6.8.1 sources).
Comment 1 Josh Triplett 2005-05-05 00:06:39 UTC
*** Bug 1479 has been marked as a duplicate of this bug. ***
Comment 2 Daniel Stone 2007-02-27 01:24:16 UTC
Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.
Comment 3 Ryan Hajdaj 2008-05-22 20:21:23 UTC
Created attachment 16700 [details]
src/Selection.c

HandleSelectionEvents() (SelectionRequest case) has been modified for balanced "EndProtectedSectioning".

Note: This "patch" is based on libXt-1.0.4 from the releases section.
Comment 4 Ryan Hajdaj 2008-05-23 07:05:06 UTC
Created attachment 16706 [details]
Diff of HandleSelectionEvents (base is libXt-1.0.4)

Here's the diff of the change (again, based on libXt-1.0.4).
Note: I was not able to find a problem with the loop logic, namely since whenever GetConversion returns true, it has always called StartProtectedSection.

The potential fix here is to call StartProtectedSection in the non-for-loop if so that there is again a balance of Start/StopProtectedSection calls.
Comment 5 Pete Zakel 2010-06-23 17:10:45 UTC
We have discovered we are having numerous crashes in our software due to this bug.  We have implemented a sledgehammer approach of continually resetting the X error handler in our event loop, but having this fix actually made to the code would solve the crashes without the workaround.

Is there any possibility that this fix can actually be made to the released code base?
Comment 6 Alan Coopersmith 2010-09-23 18:16:12 UTC
Sorry about the delay - libXt is mostly unmaintained these days since so
little uses it and no one involved knows much about it.   (Of course, should
anyone who knows much about it and needs it to work for their software want
to join X.Org to help maintain this library, we're always looking for more
volunteers.)

I can't claim to know enough about this fix to review it, but it seems 
simple enough that I've gone ahead and pushed it to git master anyway 
so others can start testing. 

Patches are generally handled most quickly when submitted as actual 
context diffs, not replacement source files or HTML dumps - see
http://www.x.org/wiki/Development/Documentation/SubmittingPatches
for guidelines.


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.