Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure all memory held by handles is freed #1711

Merged
merged 3 commits into from
Oct 6, 2020

Conversation

achamayou
Copy link
Member

Spotted after looking at https://github.com/microsoft/CCF/security/code-scanning/575?query=ref%3Arefs%2Fheads%2Fmaster, with @eddyashton's help, some of the memory held by close_ptr wasn't freed because we weren't giving the loop enough iterations to trigger close callbacks.

This purely an on-shutdown issue, not a leak during normal operation, but it's still a leak.

@achamayou achamayou requested a review from a team as a code owner October 6, 2020 10:56
@eddyashton
Copy link
Member

Strongly recommend this setting for anyone reviewing this:

image

@ccf-bot
Copy link
Collaborator

ccf-bot commented Oct 6, 2020

fix_minor_handle_leak@13735 aka 20201006.13 vs master ewma over 50 builds from 13166 to 13729
images

@achamayou
Copy link
Member Author

This triggers the following error in virtual mode:

41: #18   Object "[0xffffffffffffffff]", at 0xffffffffffffffff, in 
41: #17   Object "./cchost.virtual", at 0x408569, in 
41: #16   Object "/lib/x86_64-linux-gnu/libc.so.6", at 0x7f7765797b96, in __libc_start_main
41: #15   Object "./cchost.virtual", at 0x423f39, in 
41: #14   Object "/usr/lib/x86_64-linux-gnu/libuv.so.1", at 0x7f776717acc7, in uv_run
41: #13   Object "/usr/lib/x86_64-linux-gnu/libuv.so.1", at 0x7f776718a33f, in uv__io_poll
41: #12   Object "/usr/lib/x86_64-linux-gnu/libuv.so.1", at 0x7f776718533b, in 
41: #11   Object "/usr/lib/x86_64-linux-gnu/libuv.so.1", at 0x7f77671845ce, in 
41: #10   Object "./cchost.virtual", at 0x53c0e2, in 
41: #9    Object "./cchost.virtual", at 0x53c9bf, in 
41: #8    Object "./cchost.virtual", at 0x5c9278, in 
41: #7    Object "./cchost.virtual", at 0x5ca17e, in 
41: #6    Object "./cchost.virtual", at 0x5caf16, in 
41: #5    Object "./cchost.virtual", at 0x5caf70, in 
41: #4    Object "./cchost.virtual", at 0x5cb03b, in 
41: #3    Object "./cchost.virtual", at 0x4f6f66, in 
41: #2    Object "./cchost.virtual", at 0x4ef8d8, in 
41: #1    Object "./cchost.virtual", at 0x4ed1d5, in 
41: #0    Object "./cchost.virtual", at 0x4ed83c, in 
41: Segmentation fault (Invalid permissions for mapped object [0x7f7660416330])

src/host/main.cpp Outdated Show resolved Hide resolved
@achamayou
Copy link
Member Author

The seg fault was caused by queued up on_reads showing up after the writer had been destroyed, that's solved by the scope change. We also now correctly free the loop itself, which we weren't doing before.

The break in the Daily is #1701, which I'll look at next.

@achamayou achamayou merged commit 9bfe840 into microsoft:master Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants