MasscollabsGamingStudios/gnome-chess
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix various mistakes in ChessEngine

Browse source 
fds can be 0 (hi stdin!). Various resource leaks due to not calling
stop() when the engine stops itself. Be careful to unset all values from
the previous engine in stop(), since they're now checked to ensure
they're not overwritten as a precondition of start(). Expect
engine_stopped_cb() to be called after stop().

https://bugzilla.gnome.org/show_bug.cgi?id=724504
This commit is contained in:
Michael Catanzaro 2015-07-19 13:32:19 -05:00
parent cf425b02bd
commit fe2a655aa9
1 changed files with 23 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
src/chess-engine.vala
View file

@ -19,8 +19,8 @@ public abstract class ChessEngine : Object
private uint pending_move_source_id; private uint pending_move_source_id;
private Pid pid = 0; private Pid pid = 0;
private int stdin_fd = 0; private int stdin_fd = -1;
private int stderr_fd = 0; private int stderr_fd = -1;
private IOChannel? stdout_channel; private IOChannel? stdout_channel;
private uint stdout_watch_id = 0; private uint stdout_watch_id = 0;
private bool started = false; private bool started = false;
@ -60,8 +60,8 @@ public abstract class ChessEngine : Object
public bool start () public bool start ()
requires (pid == 0) requires (pid == 0)
requires (stdout_watch_id == 0) requires (stdout_watch_id == 0)
requires (stdin_fd == 0) requires (stdin_fd == -1)
requires (stderr_fd == 0) requires (stderr_fd == -1)
requires (!started) requires (!started)
{ {
string[] argv = {binary}; string[] argv = {binary};
@ -104,12 +104,16 @@ public abstract class ChessEngine : Object
} }
private void engine_stopped_cb (Pid pid, int status) private void engine_stopped_cb (Pid pid, int status)
requires (pid == this.pid)
requires (pid != 0)
{ {
Process.close_pid (pid); // This function could be called because the engine quit on its own, or
this.pid = 0; // it could be called because we killed the engine ourselves in
stopped (); // ChessEngine.stop(). If it quit on its own, we need to clean up here.
if (started) {
stop (false);
// This signal is only to be emitted when the chess engine stops
// itself, not when another class calls ChessEngine.stop ().
stopped ();
}
} }
public abstract void start_game (); public abstract void start_game ();
@ -140,14 +144,15 @@ public abstract class ChessEngine : Object
do_undo (); do_undo ();
} }
public void stop () public void stop (bool kill_engine = true)
requires (!started || stdout_channel != null) requires (!started || stdout_channel != null)
requires (!started || stdin_fd != 0) requires (!started || stdin_fd != -1)
requires (!started || stderr_fd != 0) requires (!started || stderr_fd != -1)
requires (!started || pid != 0) requires (!started || pid != 0)
{ {
if (!started) if (!started)
return; return;
started = false;
// This can be unset on errors in read_cb. // This can be unset on errors in read_cb.
if (stdout_watch_id != 0) if (stdout_watch_id != 0)
@ -161,17 +166,20 @@ public abstract class ChessEngine : Object
{ {
warning ("Failed to close channel to engine's stdout: %s", e.message); warning ("Failed to close channel to engine's stdout: %s", e.message);
} }
stdout_channel = null;
if (FileUtils.close (stdin_fd) == -1) if (FileUtils.close (stdin_fd) == -1)
warning ("Failed to close pipe to engine's stdin: %s", strerror (errno)); warning ("Failed to close pipe to engine's stdin: %s", strerror (errno));
stdin_fd = -1;
if (FileUtils.close (stderr_fd) == -1) if (FileUtils.close (stderr_fd) == -1)
warning ("Failed to close pipe to engine's stderr: %s", strerror (errno)); warning ("Failed to close pipe to engine's stderr: %s", strerror (errno));
stderr_fd = -1;
if (Posix.kill (pid, Posix.SIGTERM) == -1) if (kill_engine && Posix.kill (pid, Posix.SIGTERM) == -1)
warning ("Failed to kill engine: %s", strerror (errno)); warning ("Failed to kill engine: %s", strerror (errno));
Process.close_pid (pid);
started = false; pid = 0;
} }
private bool read_cb (IOChannel source, IOCondition condition) private bool read_cb (IOChannel source, IOCondition condition)