unsafe use of hash_search(... HASH_ENTER ...)

From: "Qingqing Zhou" <zhouqq(at)cs(dot)toronto(dot)edu>
To: pgsql-hackers(at)postgresql(dot)org
Subject: unsafe use of hash_search(... HASH_ENTER ...)
Date: 2005-05-27 03:27:15
Message-ID: d7646e$lqi$1@news.hub.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

-- First part ---

In md.c/RememberFsyncRequest():

if (hash_search(pendingOpsTable, &entry, HASH_ENTER, NULL) == NULL)
ereport(FATAL,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));

pendingOpsTable uses "MdCxt" to allocate memory. So if "out of memory", we
actually have no chance to raise the error level to FATAL. A quick fix is to
use malloc() HASH_ALLOC method for pendingOpsTable.

In general, code snippet like this:

if (hash_search(..., HASH_ENTER, ...) == NULL)
action_except_elog__ERROR__;

are considered unsafe if: (1) the allocation method of the target hash table
could elog(ERROR) themselves and (2) the reaction to the failure of
hash_search() is not elog(ERROR).

So shared memory hash table is safe because of condition (1). I scratched
the server code and find the following places are like this:

* RememberFsyncRequest() - solution as above;
* XLogOpenRelation() - not a problem, since it is already in the critical
section;
* IndexNext() in 8.0.1;

-- Second part ---

Also, per discussion with Neil and Tom, it is possible to simplify code
snippets like this:

if (hash_search(local_hash, HASH_ENTER, ...) == NULL)
elog(ERROR, "out of memory");

To

hash_search(local_hash, HASH_ENTER, ...);

Comments?

Regards,
Qingqing

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2005-05-27 03:40:13 Re: unsafe use of hash_search(... HASH_ENTER ...)
Previous Message Qingqing Zhou 2005-05-27 03:22:10 Re: Can we simplify win32 threading code