Opened 20 years ago

Closed 20 years ago

Last modified 20 years ago

#1745 closed defect (fixed)

FT: Xlib async errors in INSANE sequences

Reported by: SF/cloney Owned by: eriktorbjorn
Priority: blocker Component: Engine: SCUMM
Version: Keywords:
Cc: Game: Full Throttle

Description

Any attempts to control INSANE bike-fighting sequences with the keyboard cause ScummVM to freeze, issuing the following error message:

Xlib: unexpected async reply (sequence 0x7d)!

The hex number varies, but the rest is always the same. ScummVM then requires a kill -9 to get rid of.

Attached is a save game next to the collapsed fertilizer tanker. Heading up the road and trying to use the keys causes the crash. I've tested a new game from fresh, and the same problem occurs.

For the record, I'm using x.org X11 6.7.0-r2 (Gentoo revision) on a 2.6.8 Kernel with nVidia graphics. This problem has been around for ages though, and I had the same issues back when I was using XFree, but didn't get around to filing a bug report.

Hope this helps.

Cloney

Ticket imported from: #1018588. Ticket imported from: bugs/1745.

Attachments (3)

ft.s04 (11.2 KB ) - added by SF/cloney 20 years ago.
Save game just before road section.
smush.diff (4.8 KB ) - added by eriktorbjorn 20 years ago.
Patch against an April 2 CVS snapshot
updated.diff (5.0 KB ) - added by Kirben 20 years ago.
Updated patch for current ScummVM cvs

Download all attachments as: .zip

Change History (31)

by SF/cloney, 20 years ago

Attachment: ft.s04 added

Save game just before road section.

comment:1 by fingolfin, 20 years ago

Status: newpending

comment:2 by fingolfin, 20 years ago

To process your bug report appropriately, we need you to provide the following additional information:

* ScummVM version (PLEASE test the latest CVS/Daily build) * Bug details, including instructions on reproducing it * Language of game (English, German, ...) * Version of game (talkie, floppy, ...) * Platform and Compiler (Win32, Linux, MacOS, ...) * Attach a save game if possible * If this bug only occurred recently, please note the last version without the bug, and the first version including the bug. That way we can fix it quicker by looking at the changes made.

This should only take you a little time but will make it much easier for us to process your bug report in a way that satisfies both you and us.

Thank you for your support!

comment:3 by SF/cloney, 20 years ago

Status: pendingnew

comment:4 by SF/cloney, 20 years ago

Sorry, it's been too long since I did this.

ScummVM anon-CVS homebuild, dated day of bug report (I update daily) Gentoo Linux, 2.6.8, gcc-3.3.4-r1 English CD Full Throttle

Bug first appeared a while ago - around the time my previous FT bug report (957640) was resolved. I've been too busy to file.

comment:5 by sev-, 20 years ago

Yes, this bug has been here since rewrote of diMUSE started. It's some race condition between INSANE and IMUSE. Probably aquadran or fingolfin could look into it.

comment:6 by fingolfin, 20 years ago

Owner: set to aquadran
Priority: normalhigh

comment:7 by fingolfin, 20 years ago

I'll be away for two weeks. In the meantime, aquadran should be aware of this issue :-)

comment:8 by fingolfin, 20 years ago

I just fixed a SMUSH/INSANE race condition (I think), no idea if that will affect this bug, but if anybody wants to do a retest... :-)

comment:9 by sev-, 20 years ago

It became some more bulletproof, still I have "Xlib: unexpected async reply" message when hitting ESC a lot on mine road.

Once I got this: Assertion failed: (offset == 0), function reinit, file scumm/smush/chunk.cpp, line 159.

It reminds me that another Win-only bug when another variable was overwritten with zero.

I think we should carefully count bytes in all decompression routines in SMUSH code.

comment:10 by fingolfin, 20 years ago

Actually, that assert you encountered was newly added by me, read the comment next to it. It's there because the original code (added by you? not sure) was ignoring a parameter... so this particular thing is not necessarily an overwrite problem

comment:11 by sev-, 20 years ago

Yes, I added that code, but I am sorry I am too dumb to understand your comment there. May you, please, explain it some more?

by eriktorbjorn, 20 years ago

Attachment: smush.diff added

Patch against an April 2 CVS snapshot

comment:12 by eriktorbjorn, 20 years ago

With the assertion and invalid seek problems hopefully fixed this time, it would be nice if we could get this one fixed too, because I've been seeing it as well.

The only thing I'm aware of that SMUSH and INSANE do - and I've only ever seen the problem during INSANE scenes - is that they access the graphics functions in the backend from more than one thread. Could this be a problem? I know there's a mutex in place, but I still took the time to investigate who calls what. (I have assumed that there are only two threads involved - the main thread and the SMUSH thread - but I haven't actually verified that.)

The copyRectToScreen() function is called from the other thread frequently, but that happens even during normal SMUSH scenes, and I've never seen those crash. On the other hand, is there any reason we don't call it from the main thread instead? I thought it was updateScreen() that did most of the heavy lifting, not copyRectToScreen().

The updateScreen() function is called from the main thread, except where it's called because of smush_warpMouse(). (Does it really have to call updateScreen() when the mouse cursor isn't visible?)

The setPalette() function is called from both threads, but the calls from the main thread are few and far between. E.g. pressing Escape causes it to be called from the main thread, but not much else. This function is not guarded by any mutex, but perhaps it doesn't need to be?

It would be pretty simple to move all these graphics calls to the main thread instead. Would it be a good idea to do so? I'm attaching a patch against yesterday's CVS snapshot, if anyone's interested.

comment:13 by eriktorbjorn, 20 years ago

Figures. Yesterday, while trying to figure out the "invalid seek" bug, I ran into this problem half a dozen times or so. Today, with the "invalid seek" bug finally having been fixed, I've been unable to reproduce this.

Still, I don't dare to close this bug just yet. It might be one of those problems that depend on the phase of the moon...

comment:14 by SF/svena, 20 years ago

Still seeing this with snapshot as of 20050429. Seems harder to reproduce tough.

comment:15 by sev-, 20 years ago

It is fixed now thanks to Fingolfin and Kirben

comment:16 by sev-, 20 years ago

Resolution: fixed
Status: newclosed

comment:17 by Kirben, 20 years ago

This issue still occuers and is not fixed.

comment:18 by Kirben, 20 years ago

Priority: highblocker
Resolution: fixed
Status: closednew

comment:19 by sev-, 20 years ago

And after investigations it still occurs even for me under valgrind

comment:20 by fingolfin, 20 years ago

Does valgrind log anything useful? Probably not...

I wonder if there is something like an "xlib debugger". Or maybe one can somehow set a breakpoint into the xlib code that produces that error? Hrm...

comment:21 by fingolfin, 20 years ago

Priority: blockerhigh

comment:22 by eriktorbjorn, 20 years ago

http://www.faqs.org/faqs/x-faq/part7/section-15.html explains what that kind of messages mean, but we already guessed as much.

See my comment below for some note about where we call graphics-related functions from outside the main thread. Did I miss any?

(The patch probably doesn't apply any more, and I don't know if it did any good in the first place.)

comment:23 by Kirben, 20 years ago

There are no useful warnings under valgrind.

Why did you change back priority ? this is a critical issue, since the error happens frequently under linux and it is worse than a crash (Since ScummVM needs to be killed off). Basically INSANE in Full Throttle is unusble under linux and I doubt anyone could complete the game at the moment.

comment:24 by Kirben, 20 years ago

Priority: highblocker

comment:25 by Kirben, 20 years ago

Attached updated version of eriktorbjorn's patch, against current ScummVM cvs.

by Kirben, 20 years ago

Attachment: updated.diff added

Updated patch for current ScummVM cvs

comment:26 by Kirben, 20 years ago

Added eriktorbjorn's patch to ScummVM CVS, so far no more errors.

comment:27 by fingolfin, 20 years ago

Kirben: I restored the priority *back* to the original value of "7" after you silently changed, so I think I could just as well ask you why you keep changing it. Please do not change priorities w/o stating in a tracker comment that you did so, and why. The previous descriptions sounded very different from what you say in one of your most recent comments (you call FT unususable on Linux, previous reports suggested that only when you use Valgrind to force the issue, you get problems).

Erik, I am glad to hear your patch fixes the issue.

comment:28 by fingolfin, 20 years ago

Owner: changed from aquadran to eriktorbjorn
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.