Обсуждение: Avoid handle leak (src/bin/pg_ctl/pg_ctl.c)

On Thu, Oct 23, 2025 at 09:51:14PM -0300, Ranier Vilela wrote:
> The function *CreateRestrictedProcess* is responsible to create a
> restricted token
> Coverity complains that the handle origToken can be leaked.
>
> In case of failure of the functions *AllocateAndInitializeSid* or
> *GetPrivilegesToDelete*
> the handle origToken must be released.

pg_ctl exits quickly when a failure of CreateRestrictedProcess()
happens, hence why does it matter to close these handles as an exit()
should do the job as well?
--
Michael

On 2025-Oct-24, Ranier Vilela wrote:

> Handles are a scarce Windows resource.
> The work of freeing these resources is not done by exit(), but by
> Windows itself, when possible.
> If applications are not good citizens, these resources will eventually
> run out.

Hmm?  That makes no sense.  Do you have references to documentation that
says the system works the way you claim?

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Pensar que el espectro que vemos es ilusorio no lo despoja de espanto,
sólo le suma el nuevo terror de la locura" (Perelandra, C.S. Lewis)

On 10/24/2025 7:40 AM, Ranier Vilela wrote:
> 
> 
> Em sex., 24 de out. de 2025 às 09:24, Álvaro Herrera 
> <alvherre@kurilemu.de <mailto:alvherre@kurilemu.de>> escreveu:
> 
>     On 2025-Oct-24, Ranier Vilela wrote:
> 
>      > Handles are a scarce Windows resource.
>      > The work of freeing these resources is not done by exit(), but by
>      > Windows itself, when possible.
>      > If applications are not good citizens, these resources will
>     eventually
>      > run out.
> 
>     Hmm?  That makes no sense.  Do you have references to documentation that
>     says the system works the way you claim?
> 
> There are a bunch of messages on the web.
> Some are:
> "insufficient system resources exist to complete the requested service"
> 
> resource-exhaustion-detected-windows-11 <https://learn.microsoft.com/en- 
> us/answers/questions/4019236/resource-exhaustion-detected-windows-11>
> 
> fatal-error-out-of-system-resources <https://learn.microsoft.com/en-us/ 
> answers/questions/2683708/fatal-error-out-of-system-resources>
> 
> out-of-memory-error-windows/ <https://appuals.com/out-of-memory-error- 
> windows/>
> 
> best regards,
> Ranier Vilela
Currently handles have ~10000 hard limit for a process. Your processes 
handle table is allocated by the kernel in the non-paged pool of kernel 
memory.  The reason for the limit to handles is obvious, you can't have 
something growing out of control in kernel memory, especially non-paged 
pool memory. So, the bad part of a handle memory leak comes from 
creating to many handles to the point that you fill up the process 
handle table and can't create anymore. That can only happen while your 
process is running.  As soon as you call exit() the OS will get rid of 
your processes handle table in the kernel and eradicate your address 
space.  The kernel object that your handle may have been "pointing" 
(it's really just an index into your handle table) at may still exist if 
it has been shared with another process--but that is a different handle 
in a different handle table.

Currently this code is only called when running as a service.  It will 
quickly exit in the leaking path and the exit process does indeed 
cleanup the handles.  Handle leaks in userland only exist during the 
life of the process. I can not imagine how this code would consume 10000 
handles before exiting and the OS cleaning up.

Having said that, there are other things in this code that could use 
cleaning up and I am not opposed to closing the handle in the right 
spots being added during that cleanup.  The PowerUser is no longer any 
different than a normal user on any version of Windows we support.  In 
https://www.postgresql.org/message-id/e4025275-0f97-4a3e-b107-a85e60ccf0f7%40gmail.com 
the patch I attached has a static HANDLE create_restricted_token(void) 
function that does call CloseHandle(origToken) in the correct places, as 
well as removing the PowerUser specific code. The removal of the 
PowerUser and Closing the handle might be of more interest as a patch? 
But, In the words of Raymond Chen 
(https://devblogs.microsoft.com/oldnewthing/20120105-00/?p=8683), "All 
this anal-rententive memory management is pointless. The process is 
exiting. All that memory will be freed when the address space is 
destroyed. Stop wasting time and just exit already."

BG