A player thread may sleep on input or output, except:
(1) While it is executing a C_MOD command, it may only sleep on input.
(2) While it is being aborted by the update or shutdown, it may not
sleep at all.
To find out whether a player thread may sleep on input, code has to
check condition (2). It needs do to that in recvclient().
To find out whether it may sleep on output, it has to check both
conditions. It needs to do that in pr_player() and upr_player().
The code tracked condition (1) in global variable play_lock_wanted.
It checked condition (2) by examining struct player member command.
Replace all that by new struct player member may_sleep. Initialize it
in player_new(), update it in dispatch(), shutdwn() and update_run().
This makes the tests in recvclient(), pr_player() and upr_player()
obvious. play_wrlock_wanted() is now unused, remove it.
Player threads may only sleep under certain conditions. In
particular, they must not sleep while a command is being aborted by
the update or shutdown.
io.c should not know about that. Yet io_output_all() does, because it
needs to give up when update or shutdown interrupt it. The function
was introduced in Empire 2, but it didn't give up then. Fixed in
commit a7fa7dee, v4.2.22. The fix dragged unwanted knowledge of
command abortion into io.c.
To clean up this mess, io_output_all() has to go.
First user is io_write(). io_write() automatically flushes the queue.
In wait-mode, it calls io_output_all() when the queue is longer than
the bufsize, to attempt flushing the queue completely. In
no-wait-mode, it calls io_output() every bufsize bytes. Except the
test for that is screwy, so it actually misses some of the flush
conditions.
The automatic flush makes io_write() differ from io_gets(), which is
ugly. It wasn't present in BSD Empire 1.1. Remove it again, dropping
io_write()'s last argument.
Flush the queue in its callers pr_player() and upr_player() instead.
Provide new io_output_if_queue_long() for them. Requires new struct
iop member last_out to keep track of queue growth. pr_player() and
upr_player() call repeatedly until it makes no more progress. This
flushes a bit less eagerly in wait-mode, and a bit more eagerly in
non-wait mode.
Second user is recvclient(). It needs to flush the queue before
potentially sleeping in io_input(). Do that with a simple loop around
io_output(). No functional change there.
Return number of bytes written on success, -1 on error. In
particular, return zero when nothing was written because the queue was
empty, or because the write slept and got woken up, or because the
write refused to sleep.
Before, it instead returned the number of bytes remaining to be
written when empth_select() failed, when woken up from sleep, or
refusing to sleep. You couldn't tell from the return value whether
the call made progress writing out the queue.
The current callers don't actually notice the change.
Don't set IO_EOF when writev() returns zero. I don't think this could
happen, but it's wrong anyway, because a short write should not stop
future reads.
The blocking I/O option makes no sense in the server, because it
blocks the server process instead of the thread. In fact, it's been
unused since Empire 2, except for one place, where it was used
incorrectly, and got removed in the previous commit.
Make I/O non-blocking in io_open() unconditionally. Remove IO_NBLOCK
and io_noblocking().
The call switched the connection with the player to blocking I/O for
draining of output before closing the connection. Looks scary,
because blocking on I/O blocks the complete server process, not just
the player thread. But we don't do input, and we do output only with
IO_WAIT, which can't block. So this has no effect.
Chainsaw used this together with the notify callback to make the iop
data type usable for sockets it listened on, so that io_select() could
multiplex them along with the sockets used for actual I/O.
io_select() became unused in Empire 2, and finally got removed in
commit 875d72a0, v4.2.13. That made the IO_NEWSOCK and the notify
callback defunct. The latter got removed in commit 7d5a6b81, v4.3.1.
Fix empth_sleep() for argument in the past and pthreads
Calculation of sleep duration suffered integer underflow for unsigned
time_t and arguments in the past. This made empth_sleep() sleep for
"a few" years instead of not at all.
F_GETFL always failed with WSAEINVAL. io_noblocking() always failed
without doing anything. Callers didn't check for failure, and newly
opened sockets remained blocking. But because because
WSAEventSelect() makes sockets non-blocking automatically, they became
non-blocking soon enough to keep things working.
Remove the broken code to query the non-blocking state, and just
return 0. Document why this works.
While there, simplify the F_SETFL case by using ioctlsocket() instead
of WSAIoctl().
Replace the fixed $1 per ETU maintenance for capital/city sectors that
are at least 60% efficient by a configurable maintenance cost, payable
regardless of efficiency. The only change in the default
configuration is that inefficient capitals now pay maintenance.
Charging sector maintenance regardless of efficiency is consistent
with unit maintenance.
New struct dchrstr member d_maint and sector-chr selector maint. Make
show_sect_build() show it. Change produce_sect() to record
maintenance in new slot p_sect[SCT_MAINT] instead of abusing
p_sect[SCT_CAPIT]. Replace the "Capital maintenance" line in budget
by "Sector maintenance".
Print sector type mnemonic and name, like show sect s and c. Print
"can't" instead of negative number for sectors players can't designate
(this was not an issue before the previous commit). Show build cost
per 100%, like show ship, land, plane and nuke. Size the columns more
sensibly.
Fix show sect b not to omit undesignatable sectors
show sect b needs to explain any sector players can build.
show_sect_build() omitted sectors players can't designate. That's
wrong, because players can certainly own and thus build sectors they
can't designate. Test for infinite mobility cost instead, like
show_sect_stats().
Clean up automatic supply leftovers in ship update
Commit 7da69c92 (v4.3.20) removed use of automatic supply from
prod_ship(). It removed bp_enable_cachepath(), but left behind the
final bp_disable_cachepath(); bp_clear_cachepath(). Clean that up.
Fix mine production resource limit for peffic != 100%
With etu_per_update large and resource depletion quick, a sector can
produce more work than is required to fully deplete a mine. In that
case, produce() and prod() limit production to what is actually in the
ground. Except produce() got it wrong for sector types with
production efficiency other than 100%.
This affects mountains in the stock game, but only with impractically
large etu_per_update.
configure checked for library functions with LIBS instead of
LIBS_server, which could break detection of getaddrinfo() on systems
where LIB_SOCKET isn't empty.
GNUmakefile put @PTHREAD_LIBS@ only in LDLIBS, which breaks linking of
server and possibly client on systems where it is not empty.
We use the C run-time, so we better use its _beginthread(), too.
CreateThread() can lead to deadlocks, at least with some versions of
the C run-time. Broken in commit f082ef9f, v4.3.11.
Fix Windows client's stdin read thread's error handling
stdin_read_thread() zeroed bounce_status on failure, effectifely
treating it like EOF. Fix by setting to -1.
It treated main thread termination like failure, and set bounce_error
to a bogus value. Can't happen, because the program terminates when
the main thread terminates, and the only user of bounce_error is the
main thread anyway. Regardless, handle the case by terminating,
because that's more obviously correct.
Make Windows client read password without echo again
Commit 8c3b8d10 replaced the getpass() for Windows by a generic
ersatz_getpass(). This lost the "switch off echo" feature, with the
excuse that it doesn't work for me (MinGW & Wine). Turns out it works
under real Windows. Restore the feature.
The old upstream version carries the original BSD license, which is
incompatible with the GPL. Fix by rebasing to a version that is
licensed under the 2-clause BSD license.
Fix return value of s_commod() when it can't draw enough
s_commod() could incorrectly claim success when the sink ended up with
at least as many supplies than were missing initially. This caused a
number of problems:
* shp_torp() let a ship with two shells fire a torpedo, resulting in
-1 shells, which then made item_prewrite() oops. Affected missions
and return fire, but not the torpedo command.
* shp_missile_defense() let a ship with one shell use missile defense,
resulting in -1 shells, and the same item_prewrite() oops.
* Land units were considered in supply even when they had not quite
enough supplies. Such land units could defend without penalty,
attack and react. Commands load and lload weren't affected, because
they use lnd_in_supply(), which doesn't use s_commod().
upd_plane() upd_land() and left planes and land units lost to lack of
maintenance on their carriers. Cargo lists were fine anyway, because
unit_cargo_init() ignored dead units. But when the dead unit got
reused for building a new one, pln_prewrite() / lnd_prewrite() got
confused and attempted to take it off its carrier, which made
clink_rem() oops, because the unit wasn't on the cargo list. No real
harm done, as oops recovery was fine.
Fix upd_plane() and upd_land() to clear the carrier. Make
unit_cargo_init() oops when it finds dead units on carriers.
Checking Windows libraries with autoconf is cumbersome, because
linking often fails unless you include the header, and AC_SEARCH_LIBS
doesn't permit that.
Just detect the Windows API instead, with new MY_WINDOWS_API.
Should be more portable to modern systems and could be less portable
to obsolete systems than the traditional sys/time.h sys/types.h
unistd.h incantation.
getpass() is traditional Unix, but has been withdrawn from POSIX. As
such, it may be missing. Check for that, and provide ersatz. It's
not a real replacement, because it doesn't do the special magic
getpass() is supposed to do: read from /dev/tty without echo.
This bypasses our existing getpass() for Windows. In contrast to the
portable getpass(), the Windows one tries to turn off echo, but that
doesn't work for me (MinGW & Wine). Remove it.
Fix client not to hang when EOF on stdin overtakes C_EXECUTE
Player input may overtake batch file contents (well-known protocol
flaw, see doc/clients-howto for details). This includes EOF. When
that happens, the client closes standard input, sends an EOF cookie,
and continues reading output until the server closes the connection.
When it gets C_EXECUTE, it redirects input to the batch file. But it
then failed to read the batch file. The server waited forever for the
execute's EOF cookie, the client waited forever for the server closing
the connection.
Fix by stopping only reading from standard input. Broken in 8b7d0b91,
v4.3.11.
Note that the EOF cookie still overtakes the batch file contents,
which makes the server interpret the input between the execute command
and the EOF as batch file, and the batch file contents as ordinary
input.
Seamines and landmines share storage. Sea and bridge span sectors can
hold only sea mines, other sector types only landmines. Sector type
checks were missing or incorrect in several places:
* Seamines under bridge spans were mistaken for landmines in several
places:
- ground combat mine defense bonus, in get_mine_dsupport() and
stre(),
- land units retreating from bombs, in retreat_land1(),
- non-land unit ground movement (commands explore, move, transport,
and INTERDICT_ATT of military), in check_lmines(),
Fix them to check the sector type with new SCT_MINES_ARE_SEAMINES(),
SCT_LANDMINES().
* plane_sweep() mistook landmines for seamines in harbors. Bug could
not bite, because it's only called for sea sectors. Drop the bogus
check for harbor.
* Collapsing a bridge tower magically converted landmines into
seamines. Make knockdown() clear landmines.
Also use SCT_MINES_ARE_SEAMINES() and SCT_LANDMINES() in mine(),
landmine(), lnd_sweep() and lnd_check_mines(). No functional change
there.
Keep checking only for sea in pln_mine(), plane_sweep(),
retreat_ship1(), shp_sweep() and shp_check_one_mines(). This means
seamines continue not to work under bridges. Making them work there
is tempting, but as long as finding seamines clobbers the sector
designation in the bmap, it's better to have them in sea sectors only.
Historical notes:
Mines started out simple enough: you could mine sea and bridge spans,
and ships hit and swept mines in foreign sectors.
Chainsaw 2 introduced aerial mining and sweeping. Unlike ships,
planes could not mine bridge spans. plane_sweep() could sweep
harbors, which was wrong, but it was never called there, so the bug
could not bite.
Chainsaw 3 introduced landmines. The idea was to permit only seamines
in some sector types, and only landmines in the others, so they can
share storage. To figure out whether a sector has a particular kind
of mines, you need to check the sector type. Such checks already
existed in mine, drop and sweep, and they were kept unchanged. The
new lmine command also got the check. Everything else did not.
Ground movement and combat could hit and sweep seamines in bridge
spans. Ships could hit and sweep landmines in harbors.
Empire 2 fixed land unit movement (march, INTERDICT_ATT) not to
mistake seamines for landmines on bridge spans. It fixed ships not to
mistake landmines for seamines. The fix also neutered seamines under
bridge spans: ships could neither hit nor sweep them anymore. Both
fixes missed retreat.
Commit 5663713b (v4.3.1) made ship retreat consistent with other ship
movement.
Clean up unreadable assignments within if conditionals
Pinpointed assignments within if conditionals with spatch -sp_file
tests/bad_assign.cocci (from coccinelle-0.1.4). Cherry-picked diff
hunks affecting conditionals split over multiple lines, and cleaned
them up.
Crash dump forks a child to call abort(). abort() may flush or close
streams. This is unwelcome, because it can mess up streams in the
parent. Observed with the journal. Could theoretically also affect
commands info, read, turn, and wire; announcement expiry, and reading
of econfig and schedule.
Fix by using SIGABRT instead.
Note that flushing streams before fork() is not a sufficient fix,
because closing a stream can still move the file descriptor's file
position. Do it anyway, to ensure any buffered output is visible to
post_crash_dump_hook.
Fix bomb not to wipe out plane updates while asking for targets
The commands to fly planes read the planes into a plane list, and
write them back when they land. If a plane changes in the file while
it is in that plane list, the changes get wiped out when the plane
lands, triggering a seqno oops.
This is not an issue as long as the complete sortie runs
uninterrupted, because that code takes care to update flying planes
only through the appropriate plane list.
However, the bomb command suspends the planes on a pinpoint bombing
run mid-air over the target sector to let the player choose targets.
This lets code run that *can* update flying planes, for instance the
edit command.
Fix by aborting changed planes, taking care not to clobber the
changes.
When bombing ships with a force containing both planes with and
without capability ASW, pin_bomb() could fail to report presence of
submarines, and could refuse to bomb ships when there were only
submarines. The culprit is pin_bomb()'s check for capability ASW: it
checked whether the first plane in the plane list was capable instead
of checking whether any plane in the list was capable.
Fix a bug in attack that could wipe out land unit updates
A victorious attacker can move attacking land units into the newly
conquered sector or leave them behind. Normally, the player is asked
what to do, but when the land unit's army has already been told to
stay behind, or the command has been aborted, the land unit stays
behind without asking. In that case, a copy of the land unit made
right after the victory was written back. Any updates since the
victory were wiped out, triggering a seqno mismatch oops.
Fix by moving the re-read of the land unit in ask_move_in() out of the
prompt conditional.
check_loan_ok(), check_comm_ok() and check_trade_ok() should have been
changed to ignore timestamps when timestamps were added to their files
in commit a680c811, v4.3.12.
Don't log out player when update aborts a command with pthreads
pthread.c's empth_select() returned -1 when empth_wakeup() interrupted
select(). The failure then got propagated all the way up, and the
player got logged out. Fix by returning 0 in that case. While there,
retry on EINTR, to match LWP. Also clarify comments.