Opened 6 years ago

Closed 6 years ago

#5372 defect closed fixed (fixed)

potential leak of OVERLAPPED structures

Reported by: Antoine Pitrou Owned by: jesstess
Priority: highest Milestone:
Component: core Keywords:
Cc: Antoine Pitrou, jesstess Branch: branches/iocp-overlapped-5372
branch-diff, diff-cov, branch-cov, buildbot
Author: antoine

Description

When a socket function returns an error (other than ERROR_IO_PENDING), the iocpsupport module doesn't free the allocated OVERLAPPED structure before returning. If I'm not mistaken, this is a memory leak. Patch attached.

Attachments (2)

ovleaks.patch (2.7 KB) - added by Antoine Pitrou 6 years ago.
ovleaks2.patch (85.2 KB) - added by Antoine Pitrou 6 years ago.
Updated patch with iocpsupport.c changes.

Download all attachments as: .zip

Change History (11)

Changed 6 years ago by Antoine Pitrou

Attachment: ovleaks.patch added

comment:1 Changed 6 years ago by Antoine Pitrou

Cc: Antoine Pitrou added
Keywords: review added

comment:2 Changed 6 years ago by jesstess

Owner: set to jesstess

comment:3 Changed 6 years ago by jesstess

Cc: jesstess added

Thanks for the bug report and patch, antoine, and sorry for the delay in getting this reviewed. I think I've convinced myself via the msdn docs that your changes look good, but I'm not seeing where the OVERLAPPED structures are getting freed in the non-error case. Can you shed some light on that?

I'll stick this patch in a branch to kick off test builds, but for ease of testing can you attach the generated iocpsupport.c changes as well?

comment:4 in reply to:  3 Changed 6 years ago by Antoine Pitrou

Replying to jesstess:

Thanks for the bug report and patch, antoine, and sorry for the delay in getting this reviewed. I think I've convinced myself via the msdn docs that your changes look good, but I'm not seeing where the OVERLAPPED structures are getting freed in the non-error case. Can you shed some light on that?

When the overlapped operation is initiated successfully (i.e. returns without error or with ERROR_IO_PENDING, the system libraries have ownership of the OVERLAPPED structure. Completion is then checked using GetQueuedCompletionStatus() (in CompletionPort.getEvent in iocpsupport.pyx) which gives back the OVERLAPPED structure which has completed. This is where we finally free the structure.

(note I'm not a Win32 expert, but this is what I get following the MSDN docs)

I'm attaching a patch including iocpsupport.c changes.

Changed 6 years ago by Antoine Pitrou

Attachment: ovleaks2.patch added

Updated patch with iocpsupport.c changes.

comment:5 Changed 6 years ago by jesstess

Author: jesstess
Branch: branches/iocp-overlapped-5372

(In [33436]) Branching to 'iocp-overlapped-5372'

comment:6 Changed 6 years ago by jesstess

(In [33437]) Apply patch ovleaks2.patch by antoine

This includes the generate iocpsupport.c changes.

refs #5372

comment:7 Changed 6 years ago by Thijs Triemstra

Owner: jesstess deleted

comment:8 Changed 6 years ago by PenguinOfDoom

Keywords: review removed
Owner: set to jesstess
Priority: normalhighest

I approve.

comment:9 Changed 6 years ago by jesstess

Author: jesstessantoine
Resolution: fixed
Status: newclosed

Merge iocp-overlapped-5372

Author: antoine Reviewer: jesstess, PenguinOfDoom Fixes: #5372

Fix a leak in some error cases of OVERLAPPED structures.

Note: See TracTickets for help on using tickets.