Opened 17 years ago

Closed 17 years ago

Last modified 16 months ago

#8053 closed patch

Inappopriate error() exit approach

Reported by: SF/jamieson630 Owned by: SF/ender
Priority: normal Component: --Other--
Keywords: Cc:
Game:

Description

The error() function used far and wide across ScummVM attempts to exit the program using exit(1). This type of bailout fails to properly shut down supporting systems and often results in processor faults (seg fault, page fault, etc).

This patch replaces exit(1) with a call to the system shutdown function OSystem_SDL_Common::quit().

Note that there are 33 other active occurrences of exit(1) within ScummVM, and they may also consitute inappropriate bailouts that cause processor faults. It may be advisable to review these to see if a cleaner exit approach could be applied.

Ticket imported from: #610251. Ticket imported from: patches/158.

Attachments (1)

scummvm.diff (517 bytes ) - added by SF/jamieson630 17 years ago.
Patch against 1.16 revision

Download all attachments as: .zip

Change History (5)

by SF/jamieson630, 17 years ago

Attachment: scummvm.diff added

Patch against 1.16 revision

comment:1 by fingolfin, 17 years ago

Funny, I was talking just today about this on #scummvm :-)

Another reason why we would want orderly exit is so that we can properly clean up resources, i.e. delete the Engine/Config objects which can in turn delete all their resources -> makes it much easier to detect memory leaks, but on the negative side slows down program exits.

One possible approach would be to use exceptions for this, but I am not sure if all our targets support those (I know that some embedded C++ compilers don't). But considering that we use templates, I guess it would be OK.

Why exceptions and not just calling some common quit() method or setting a global flag? Because we may have to exit at any point, so a flag won't work always correctly, and a global quit() function might have a hard time to clean up properly. Of course it still would be possible, but excpetions will also clean up the stack... let's ponder this a bit.

comment:2 by SF/ender, 17 years ago

Owner: set to SF/ender
Status: newclosed

comment:3 by SF/ender, 17 years ago

For the moment I've just went and replaced a few of the exit()s with a call to OSystem->quit().

We can come up with a better cleanup method later :)

comment:4 by digitall, 16 months ago

Component: --Other--
Note: See TracTickets for help on using tickets.