On Fri, May 2, 2025 at 9:54 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> Thanks for the review and testing! I've fixed the issue you pointed out.
> Updated patch attached.
Thanks for addressing this. However, I believe this commit may need to
take note of the following comment from elog.h:
* Note: if a local variable of the function containing PG_TRY is modified
* in the PG_TRY section and used in the PG_CATCH section, that variable
* must be declared "volatile" for POSIX compliance. This is not mere
* pedantry; we have seen bugs from compilers improperly optimizing code
* away when such a variable was not marked. Beware that gcc's -Wclobbered
* warnings are just about entirely useless for catching such oversights.
Based on this comment, I believe in_progress must be declared volatile.
As a stylistic comment, I think I would prefer making in_progress a
file-level global and giving it a less generic name (e.g.
LogMemoryContextInProgress). However, perhaps others will disagree.
--
Robert Haas
EDB: http://www.enterprisedb.com