From: | Neil Conway <neilc(at)samurai(dot)com> |
---|---|
To: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | leaks in TopMemoryContext? |
Date: | 2006-01-11 07:45:37 |
Message-ID: | 1136965537.9143.71.camel@localhost.localdomain |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
While thinking about how to do memory management for the TupleDesc
refcounting changes, it occurred to me that this coding pattern is
dangerous:
local_var = MemoryContextAlloc(TopMemoryContext, ...);
func_call();
/* ... */
/* update global state */
if (global != NULL)
pfree(global);
global = local_var;
the idea being that we're allocating some memory that should live longer
than the current transaction. For example, see circa line 141 in
mb/mbutils.c:SetClientEncoding().
This works fine if no error occurs: presumably, the code takes care of
releasing the allocation in TopMemoryContext when appropriate (as
mbutils.c does). The problem arises if there is an error: suppose that
func_call() in the example above elogs. The elog handler will abort the
current transaction and reset per-transaction memory contexts, but
TopMemoryContext will not be reset. Therefore the memory allocated for
"local_var" will be leaked.
To see the leak in practice, insert an elog(ERROR) in mbutils.c here:
line 141:
oldcontext = MemoryContextSwitchTo(TopMemoryContext);
to_server = palloc(sizeof(FmgrInfo));
to_client = palloc(sizeof(FmgrInfo));
fmgr_info(to_server_proc, to_server);
fmgr_info(to_client_proc, to_client);
>>>>> elog(ERROR, "...");
MemoryContextSwitchTo(oldcontext);
Running a stream of "SET client_encoding = 'koi8';" on my machine yields
a steady growth in the size of the TopMemoryContext.[1]
The elog(ERROR) above is inserted by hand to make the problem easier to
reproduce, but it is certainly possible for the calls to palloc() or
fmgr_info() to fail for legitimate reasons during normal usage (e.g. OOM
in the case of palloc(), or a multitude of reasons for fmgr_info()).
I'm not sure if there's a clean way to fix the problem. One solution
would be to enclose code that does allocations in TopMemoryContext in a
PG_TRY block. We could then pfree() the local variable in a PG_CATCH
block (provided that the "global = local_var" assignment hasn't occurred
yet). Unfortunately, that's pretty ugly, and requires adding hackery to
every site where this coding pattern occurs.
Thoughts?
-Neil
P.S. It occurs to me that the mbutils code is actually busted anyway, as
it assumes that a pfree'ing the FmgrInfo is sufficient to release all
the resources allocated by fmgr_info(). However, this is not the case:
the CurrentMemoryContext when fmgr_info() is called is used to allocate
additional things. mbutils ought to invoke fmgr_info() in its own
long-lived memory context -- but that is unrelated to the problem
described above...
[1] This codepath is only followed if the server_encoding is not
compatiable with the chosen client_encoding -- in my case
server_encoding = 'en_CA.utf8' reproduces the problem.)
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2006-01-11 07:58:32 | Re: leaks in TopMemoryContext? |
Previous Message | Tom Lane | 2006-01-11 05:08:45 | Re: Is Optimizer smart enough? |