1171464 - Check that socket isn't already closed before closing it

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Fredrik Lanker [:lanker]]

We have seen cases where mSocket ends up being null in RilConsumer::Close(), leading to a crash. I think this is what's happening:

RilConsumer::RilConsumer()
StreamSocket::Connect() fails
RilConsumer::OnConnectError()
RilConsumer::Close()
StreamSocket::Close()
RilConsumer::OnDisconnect()
StreamSocket::Connect() fails again
RilConsumer::Close() continues, setting mSocket to nullptr
RilConsumer::OnConnectError()
RilConsumer::Close()
-> mSocket == nullptr -> crash!

Adding a check in RilConsumer::Close() that mSocket != nullptr fixes this.

Comment 1

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Thanks for reporting this bug. I'm currently working on refactoring this code. I'll fix the bug in the process.

Comment 2

[Fredrik Lanker [:lanker]]

Attachment: Bug_1171464-Check_that_socket_isnt_already_closed_before_closing_it.patch

Comment 3

[Fredrik Lanker [:lanker]]

OK, good.

(I didn't see your comment before uploading the patch, you're to fast... ;))

Comment 4

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 8615319 [details] [diff] [review]
Bug_1171464-Check_that_socket_isnt_already_closed_before_closing_it.patch

Review of attachment 8615319 [details] [diff] [review]:
-----------------------------------------------------------------

Well, since you have a patch we can also fix it immediately. Do you need help with landing the fix?

Comment 5

[Fredrik Lanker [:lanker]]

Yes, thank you.

Comment 6

[Thomas Zimmermann [:tzimmermann] [:tdz]]

I'll commit the patch after Kyle gave his r+.

Comment 7

[Thomas Zimmermann [:tzimmermann] [:tdz]]

https://treeherder.mozilla.org/#/jobs?repo=b2g-inbound&revision=7cc8862ffd91

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7cc8862ffd91