Opened 15 years ago

Closed 14 years ago

Last modified 14 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
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 15 years ago.
Save game just before road section.
smush.diff (4.8 KB ) - added by eriktorbjorn 15 years ago.
Patch against an April 2 CVS snapshot
updated.diff (5.0 KB ) - added by Kirben 14 years ago.
Updated patch for current ScummVM cvs

Download all attachments as: .zip

Change History (31)

by SF/cloney, 15 years ago

Attachment: ft.s04 added

Save game just before road section.

comment:1 by fingolfin, 15 years ago

Status: newpending

comment:2 by fingolfin, 15 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, 15 years ago

Status: pendingnew

comment:4 by SF/cloney, 15 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-, 15 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, 15 years ago

Owner: set to aquadran
Priority: normalhigh

comment:7 by fingolfin, 15 years ago

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

comment:8 by fingolfin, 15 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-, 15 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, 15 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-, 15 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, 15 years ago

Attachment: smush.diff added

Patch against an April 2 CVS snapshot

comment:12 by eriktorbjorn, 15 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, 15 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, 14 years ago

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

comment:15 by sev-, 14 years ago

It is fixed now thanks to Fingolfin and Kirben

comment:16 by sev-, 14 years ago

Resolution: fixed
Status: newclosed

comment:17 by Kirben, 14 years ago

This issue still occuers and is not fixed.

comment:18 by Kirben, 14 years ago

Priority: highblocker
Resolution: fixed
Status: closednew

comment:19 by sev-, 14 years ago

And after investigations it still occurs even for me under
valgrind

comment:20 by fingolfin, 14 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, 14 years ago

Priority: blockerhigh

comment:22 by eriktorbjorn, 14 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, 14 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, 14 years ago

Priority: highblocker

comment:25 by Kirben, 14 years ago

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

by Kirben, 14 years ago

Attachment: updated.diff added

Updated patch for current ScummVM cvs

comment:26 by Kirben, 14 years ago

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

comment:27 by fingolfin, 14 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, 14 years ago

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