]> granicus.if.org Git - postgresql/commit
Reduce stack space consumption in tzload().
authorTom Lane <tgl@sss.pgh.pa.us>
Thu, 7 Jul 2016 15:28:17 +0000 (11:28 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Thu, 7 Jul 2016 15:28:17 +0000 (11:28 -0400)
commit62c8421e87b33b2e94f8a7842e1dd9aa1a286ffc
tree37754d294bd73b6ab5f210c0ae8df1a7d14ef7d3
parent60d50769b70ca5030711f70c86d7a2d3bcf34963
Reduce stack space consumption in tzload().

While syncing our timezone code with IANA's updates in commit 1c1a7cbd6,
I'd chosen not to adopt the code they conditionally compile under #ifdef
ALL_STATE.  The main thing that that drives is that the space for gmtime
and localtime timezone definitions isn't statically allocated, but is
malloc'd on first use.  I reasoned we didn't need that logic: we don't have
localtime() at all, and we always initialize TimeZone to GMT so we always
need that one.  But there is one other thing ALL_STATE does, which is to
make tzload() malloc its transient workspace instead of just declaring it
as a local variable.  It turns out that that local variable occupies 78K.
Even worse is that, at least for common US timezone settings, there's a
recursive call to parse the "posixrules" zone name, making peak stack
consumption to select a time zone upwards of 150K.  That's an uncomfortably
large fraction of our STACK_DEPTH_SLOP safety margin, and could result in
outright crashes if we try to reduce STACK_DEPTH_SLOP as has been discussed
recently.  Furthermore, this means that the postmaster's peak stack
consumption is several times that of a backend running typical queries
(since, except on Windows, backends inherit the timezone GUC values and
don't ever run this code themselves unless you do SET TIMEZONE).  That's
completely backwards from a safety perspective.

Hence, adopt the ALL_STATE rather than non-ALL_STATE variant of tzload(),
while not changing the other code aspects that symbol controls.  The
risk of an ENOMEM error from malloc() seems less than that of a SIGSEGV
from stack overrun.

This should probably get back-patched along with 1c1a7cbd6 and followon
fixes, whenever we decide we have enough confidence in the updates to do
that.
src/timezone/localtime.c