Bug 28595 - xcb_io.c: record extension is broken
Summary: xcb_io.c: record extension is broken
Status: VERIFIED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Lib/Xlib (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: xorg-7.6
  Show dependency treegraph
 
Reported: 2010-06-18 02:37 UTC by Rami Ylimaki
Modified: 2010-06-22 05:39 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Modified test case from bug 27595. (1.61 KB, text/plain)
2010-06-18 02:37 UTC, Rami Ylimaki
no flags Details

Description Rami Ylimaki 2010-06-18 02:37:55 UTC
Created attachment 36357 [details]
Modified test case from bug 27595.

STEPS:
1. Run "cnee --record --mouse" or the attached minimal test case.
2. Move your mouse.

EXPECTED RESULT:
1. Cnee displays motion event data.
2. Test case prints "got something" whenever MotionNotify or other events are received.

ACTUAL RESULT:
Both Cnee and test case are silent.



Reverting commit "Fix Xlib/XCB for multi-threaded applications (with caveats)" (933aee1d5c53b0cc7d608011a29188b594c8d70b) makes recording work again.
Comment 1 Rami Ylimaki 2010-06-18 02:43:21 UTC
With latest X server you should also apply the following fix first before using record extension:
http://lists.freedesktop.org/archives/xorg-devel/2010-June/010302.html.
Comment 2 Pauli 2010-06-18 04:03:42 UTC
CCing Jamey.
Comment 3 Josh Triplett 2010-06-19 10:59:57 UTC
Jamey and I fixed this in commit c115095d7f2bc4f5a4fb26380e3698fefdad7611.  Turned out we had a bug in the async-reply handling in poll_for_response.  Nobody noticed until someone tried to use async replies, like RECORD does. :)

The commit message:

    poll_for_response: Really handle xcb_poll_for_reply getting a reply.
    
    Don't lose async replies. That's bad.
    
    `xlsfonts -l`, which uses XListFontsWithInfo, worked fine, because the
    _XReply path worked; that path waited for replies, rather than polling.
    
    However, XRecordProcessReplies, which does nothing but call XPending,
    relied on the event-handling path to process async replies, and that was
    busted.
    
    Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=28595
    
    Signed-off-by: Jamey Sharp <jamey@minilop.net>
    Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Comment 4 Rami Ylimaki 2010-06-21 04:36:56 UTC
Hi,

Record extension is now fixed, but after applying the patch, rendercheck is always dumping core.

# rendercheck 
rendercheck 1.3
Render extension version 0.10
Window format: r5g6b5
Segmentation fault (core dumped)
Comment 5 Jamey Sharp 2010-06-21 11:00:19 UTC
(In reply to comment #4)
> Hi,
> 
> Record extension is now fixed, but after applying the patch, rendercheck is
> always dumping core.
> 
> # rendercheck 
> rendercheck 1.3
> Render extension version 0.10
> Window format: r5g6b5
> Segmentation fault (core dumped)

OK, fixed that one in commit 4a8b6528ff69f6feb8c0e119939b4ce6c088f29e. Apparently we had never tested the case where an application calls _XReadEvents and gets an I/O error and doesn't have any async replies to wait for.

I'm not sure whether rendercheck is supposed to get an I/O error there, though. You'll have to tell me if that's normal. If not, please open a new bug about it.

This should have been a separate bug, by the way. It's harder to keep track of which patch fixes what if they all have the same bug number.
Comment 6 Jamey Sharp 2010-06-21 14:08:50 UTC
OK, Josh and I botched the original fix, and the followup fix I pushed was masking the symptom, not fixing the problem. So I've reverted both and pushed a commit that actually seems to work for the RECORD test case and rendercheck and my Gnome desktop. Sorry for the confusion. Let me know if I got it right this time.
Comment 7 Rami Ylimaki 2010-06-22 05:39:51 UTC
(In reply to comment #6)
> OK, Josh and I botched the original fix, and the followup fix I pushed was
> masking the symptom, not fixing the problem. So I've reverted both and pushed a
> commit that actually seems to work for the RECORD test case and rendercheck and
> my Gnome desktop. Sorry for the confusion. Let me know if I got it right this
> time.

Thanks, it seems to work now.


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.