Skip to content

Fix hard-fault when socket created using accept() is deleted #8337

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

Closed
wants to merge 1 commit into from

Conversation

sentinelt
Copy link
Contributor

Description

When socket created using accept() is deleted the destructor is called
on TCPSocket which in turn calls close() method which executes
"delete this". This causes hard-fault.

Set _factory_allocated to 0 in destructor call which effectively skips
"delete this" in a close method.

Tested on NUCLEO-F767ZI. Applies both to master and to release-candidate branches.

Pull request type

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

When socket created  using accept() is deleted the destructor is called
on TCPSocket which in turn calls close() method which executes
"delete this". This causes hard-fault.

Set _factory_allocated to 0 in destructor call which effectively skips
"delete this" in a close method.
@cmonr cmonr requested review from SeppoTakalo and a team October 8, 2018 15:32
@0xc0170
Copy link
Contributor

0xc0170 commented Oct 15, 2018

@ARMmbed/mbed-os-ipcore Please review

@SeppoTakalo
Copy link
Contributor

This fix might be correct, but before I can approve, I need to verify it with a testcase.

TCPSocket::accept() is lacking lot of testcases, which are now under development within this sprint. So once we can test this, I will reply back.

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 24, 2018

Set as "preceding PR" needed, should be tested by #8499 ?

@michalpasztamobica
Copy link
Contributor

@0xc0170 , I think that #8499 makes this pull request unnecessary.
@SeppoTakalo , should it be rejected/closed?
@sentinelt , please take a look at #8499 and let us know your thoughts.

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 24, 2018

OK waiting for confirmation

@SeppoTakalo
Copy link
Contributor

Yes, this can be closed.
We will fix the root cause issue, provide a testcase for it and submit all in #8499

@sentinelt sentinelt closed this Oct 24, 2018
@sentinelt sentinelt deleted the socket-destroyed branch October 24, 2018 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants