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.

Sign up for GitHub

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

Jump to bottom

Fix memory leak on NetworkInterface destruction #11649

Merged
merged 1 commit into from
Oct 14, 2019
Merged

Fix memory leak on NetworkInterface destruction #11649

merged 1 commit into from
Oct 14, 2019

Conversation

michalpasztamobica
Copy link
Contributor

Description

We dynamically allocate memory in every add_event_listener(), but we did not free it on NetworkInterface destruction.

Found with the unittests update in #11612

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@AnttiKauppila
@kjbracey-arm
@SeppoTakalo
@VeijoPesonen

@AnttiKauppila
Copy link

@ARMmbed/mbed-os-maintainers Please prioritise this as it fixes a potential memory leak

@ciarmcom
Copy link
Member

ciarmcom commented Oct 8, 2019

@michalpasztamobica, thank you for your changes.
@AnttiKauppila @VeijoPesonen @SeppoTakalo @kjbracey-arm @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

@AnttiKauppila
Copy link

@adbridge I already approved, but approval is gone now after robot did "ciarmcom requested review from ..."?

@adbridge
Copy link
Contributor

adbridge commented Oct 8, 2019

@AnttiKauppila there's a known issue there that I haven't had time to look at yet.

@adbridge
Copy link
Contributor

adbridge commented Oct 8, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Oct 8, 2019

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

@OPpuolitaival
Copy link
Contributor

Greentea test was CI internal problem. Now fixed and test restarted.

@michalpasztamobica
Copy link
Contributor Author

michalpasztamobica commented Oct 8, 2019 •
edited
Loading

I'm sorry, but again, I fail to see the failure...
I can see that K64F allegedly failed, but I looked through all the logs and see no test failure (always 1159 OK).

EDIT: nevermind, @OPpuolitaival , thanks for clarifying.

@OPpuolitaival
Copy link
Contributor

@michalpasztamobica see my comment

@michalpasztamobica
Copy link
Contributor Author

@ARMmbed/mbed-os-maintainers These CI checks here have been waiting for status for the last 2 days. They might need a little bit of attention.
The doxy-spellcheck log says:

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

@adbridge adbridge closed this Oct 11, 2019
@adbridge adbridge reopened this Oct 11, 2019
@michalpasztamobica michalpasztamobica force-pushed the NetworkInterface_destructor_mem_leak branch from c11aed1 to 169d718 Compare October 11, 2019 14:12
@michalpasztamobica
Copy link
Contributor Author

Pending on #11678

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 14, 2019

Pending on #11678

I'll check shortly this one and this should proceed

@michalpasztamobica
Fix memory leak on NetworkInterface destruction
3d59646 
We dynamically allocate memory in every add_event_listener(), but we do not free it on NetworkInterface destruction.
@michalpasztamobica michalpasztamobica force-pushed the NetworkInterface_destructor_mem_leak branch from 169d718 to 3d59646 Compare October 14, 2019 07:58
@michalpasztamobica
Copy link
Contributor Author

Rebased on top of the travis fix. Hopefully CI will pass now 😃

@michalpasztamobica
Copy link
Contributor Author

doxy-spellcheck passed.
@0xc0170 , would you please trigger the CI?

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 14, 2019

Ci started

@mbed-ci
Copy link

mbed-ci commented Oct 14, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

@0xc0170 0xc0170 merged commit 7d8f1b9 into ARMmbed:master Oct 14, 2019
@michalpasztamobica michalpasztamobica deleted the NetworkInterface_destructor_mem_leak branch October 14, 2019 11:17
@michalpasztamobica michalpasztamobica mentioned this pull request Oct 14, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnttiKauppila AnttiKauppila AnttiKauppila approved these changes

@adbridge adbridge adbridge approved these changes

@SeppoTakalo SeppoTakalo SeppoTakalo approved these changes

@kjbracey kjbracey Awaiting requested review from kjbracey

@VeijoPesonen VeijoPesonen Awaiting requested review from VeijoPesonen

Assignees
No one assigned
Labels
release-version: 5.14.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@michalpasztamobica @AnttiKauppila @ciarmcom @adbridge @mbed-ci @OPpuolitaival @0xc0170 @SeppoTakalo