Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 62c8421

Browse files
committed
Reduce stack space consumption in tzload().
While syncing our timezone code with IANA's updates in commit 1c1a7cb, 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 1c1a7cb and followon fixes, whenever we decide we have enough confidence in the updates to do that.
1 parent 60d5076 commit 62c8421

File tree

1 file changed

+10
-2
lines changed

1 file changed

+10
-2
lines changed

src/timezone/localtime.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -536,9 +536,17 @@ tzloadbody(char const * name, char *canonname, struct state * sp, bool doextend,
536536
int
537537
tzload(const char *name, char *canonname, struct state * sp, bool doextend)
538538
{
539-
union local_storage ls;
539+
union local_storage *lsp = malloc(sizeof *lsp);
540540

541-
return tzloadbody(name, canonname, sp, doextend, &ls);
541+
if (!lsp)
542+
return errno;
543+
else
544+
{
545+
int err = tzloadbody(name, canonname, sp, doextend, lsp);
546+
547+
free(lsp);
548+
return err;
549+
}
542550
}
543551

544552
static bool

0 commit comments

Comments
 (0)