On 2/23/2026 4:07 PM, Álvaro Herrera wrote: > On 2026-Feb-23, Bryan Green wrote: > >> I have implemented DuplicateHandle and closed the handle in the >> appropriate places. I also reset backtrace_process to NULL if >> SymInitialize() fails. Patch is attached. > > Hmm, should then backtrace_cleanup() cope with the case where it's NULL? > > Also, I wonder what happens if one "backtraceable" error occurs, and we > fail to SymInitialize(), then another backtraceable error occurs. > Should we do the DuplicateHandle()+SymInitialize() dance again, or > should we just give up? The current implementation does the former, I > think; but the latter is also easily achievable by setting > backtrace_symbols_initialized to true and leaving backtrace_process as > NULL; then this case can be detected specifically in set_backtrace() and > treated as a case where we just return NULL before attempting anything > else. > backtrace_cleanup() only cleans up on process exit-- so, it is almost a style choice. If we are going to close the handle in cleanup we should probably stay consistent and reset backtrace_process to null as well.
If SymInitialize() fails...I can not imagine it ever succeeding after that. It is an expensive call...a process generating errors shouldn't be burning time retrying a failing system call on every error. I agree with not retrying. I'll put together another patch with those changes. -- Bryan Green EDB: https://www.enterprisedb.com
