unsafe use of hash_search(... HASH_ENTER ...) - Mailing list pgsql-hackers

From Qingqing Zhou
Subject unsafe use of hash_search(... HASH_ENTER ...)
Date
Msg-id d7646e$lqi$1@news.hub.org
Whole thread Raw
Responses Re: unsafe use of hash_search(... HASH_ENTER ...)
Re: unsafe use of hash_search(... HASH_ENTER ...)
List 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




pgsql-hackers by date:

Previous
From: "Qingqing Zhou"
Date:
Subject: Re: Can we simplify win32 threading code
Next
From: Tom Lane
Date:
Subject: Re: unsafe use of hash_search(... HASH_ENTER ...)