[Freeciv-Dev] Re: Patch for civserver: save game on SIGPWR, SIGTERMor SIGHUP

[Jason Short <jshort@xxxxxxxxxxxxx>]

"Eric R. Smith" wrote:
> Mika Korhonen wrote:
> > On Thu, 3 May 2001, Eric R. Smith wrote:

> > Almost zero-probability pitholes:
> >
> > 1) what if SIGxxx is caught in the middle of saving the game
> > 2) what if signals, say SIGHUP and SIGTERM, arrive in (very
> >    close) succession
> 
> Hmmm, good points. I hadn't thought all of the cases through carefully!

Well, everyone seems to be in agreement that it's better than nothing. 
It can certainly be improved to handle these extra cases, though.

BTW, the patch doesn't seem to apply to the CVS version well at all - it
looks like all of that code has been moved into a different file.  I
think I got it figured out, though.

> I think that if case (1) occurs then the regular save file will
> be corrupted, but the *_panic.sav.gz should be created properly. I'm
> not certain of that, though -- it depends on whether there is any
> "state" in the save file code which would be disrupted if that code
> is re-entered. It also depends on what functions the save code calls;
> not all C standard functions are guaranteed to be callable from
> signal handlers when they themselves are interrupted by the signal.

Right, as long as there are no static variables (or other problems of
that kind), things should be okay.  Even if there are, though, you'd
expect them to be initialized at the beginning of the function and
things would still be okay.

> (2) is definitely a problem. It falls into "implementation defined
> behaviour"
> in the Posix standard, I think, and could mean that the process would
> be killed and a corrupted save file left. A better patch would probably
> use the "sigaction" function to install the signal handlers rather than
> "signal". "sigaction" can block other signals during the handler.
> However, I'm not sure how portable this would be, particularly to
> Windows,
> and as you say it's probably a very rare problem.

Right; sigaction could be used but probably shouldn't.  Since
freeciv-server isn't multithread, though (it's not, right?), no true
locking should be necessary.  All we need to worry about is interrupts
which will move execution to the beginning of the save_game_signal()
function; they could happen either during save_game (corrupting the
current save file) or afterwards during the exit (corrupting the action
of exiting the game).

I have a solution, patch below, that should solve both problems.  I'm
not prepared to offer a proof of its correctness, but I think it's
clean.  Certainly it is an improvement.  The code is pretty heavily
commented on why it should be correct.

Patch (made against current CVS) is at 
        http://devon.dhs.org/freeciv/save_game_on_signal.patch

I also disable the signal handlers after the main loop is over; I'm a
bit unsure of this code (is this the right time for it?), but it would
certainly be bad for an interrupt to try to save and exit while the game
was already in the process of exiting.

jason short