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

"Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes
> "Qingqing Zhou" <zhouqq(at)cs(dot)toronto(dot)edu> writes:
> > 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).
>
> I've made some changes to hopefully prevent this type of thinko again.
> Thanks for spotting it.
>

I am afraid the problem are not limited to hash_search(). Any code snippet
are not proteced by critical section like this:

Assert(CritSectionCount == 0);
ret = do_something_might_elog_error();
if (is_not_expected(ret))
action_raise_error_higher_than_ERROR;

are all need to re-considered. For example,

---
file = AllocateFile(full_path, "r");
if (!file)
{
if (errno == ENOENT)
ereport(FATAL,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("\"%s\" is not a valid data directory",
path),
errdetail("File \"%s\" is missing.", full_path)));
else
ereport(FATAL,
(errcode_for_file_access(),
errmsg("could not open file \"%s\": %m", full_path)));
}
---

AllocateFile() itself could raise an error so we increase error level to
FATAL.

Regards,
Qingqing