Avoid concurrent dhcp_do_request execution

RESOLVED FIXED in Firefox 40, Firefox OS v2.0M

Status

Firefox OS
Wifi
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: hchang, Assigned: hchang)

Tracking

unspecified
2.2 S9 (3apr)
x86
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:2.0M+, firefox38 wontfix, firefox39 wontfix, firefox40 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.1S fixed, b2g-v2.2 fixed, b2g-master fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
The implementation of |dhcp_do_request| [1] is not a reentrant function since it uses a global properties (e.g. dhcp.wlan0.xxxx) to store the result. We need to make sure not calling |dhcp_do_request| while another one is running.

[1] http://androidxref.com/4.4.4_r1/xref/system/core/libnetutils/dhcp_utils.c#234
(Assignee)

Updated

3 years ago
Assignee: nobody → hchang
(Assignee)

Comment 2

3 years ago
Hi Selin,

Could you please check if this patch mitigate the DHCP re-request issue
that we discuss yesterday? Thanks!
Flags: needinfo?(kli)
I backport to v2.0m and try this patch. It works fine.
Flags: needinfo?(kli)
(Assignee)

Comment 4

3 years ago
Comment on attachment 8582148 [details] [diff] [review]
Bug1146248.patch

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

Hi Vincent, could you please help review this patch to avoid concurrent dhcp request? Thanks!
What this patch does is pretty much the same as dhcp_stop()  [1]

http://androidxref.com/4.4.4_r1/xref/system/core/libnetutils/dhcp_utils.c#260
Attachment #8582148 - Flags: review?(vchang)
Comment on attachment 8582148 [details] [diff] [review]
Bug1146248.patch

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

Looks good. Thank you.
Attachment #8582148 - Flags: review?(vchang) → review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0ac8ff6c3bed
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S9 (3apr)
Duplicate of this bug: 1133159
Created attachment 8586001 [details] [diff] [review]
[v2.0m] Stop previous DHCP request when requesting a new one.

Henry, I rebased a patch for v2.0m branch, could you have a look? Thanks!
Attachment #8586001 - Flags: feedback?(hchang)
Josh, we need this patch for v2.0m.
blocking-b2g: --- → 2.0M?
status-b2g-v2.0M: --- → affected
status-b2g-v2.1: --- → affected
status-b2g-v2.1S: --- → affected
status-b2g-v2.2: --- → affected
status-b2g-master: --- → fixed
(Assignee)

Comment 12

3 years ago
Comment on attachment 8586001 [details] [diff] [review]
[v2.0m] Stop previous DHCP request when requesting a new one.

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

looks good! Thanks!
Attachment #8586001 - Flags: feedback?(hchang) → feedback+

Updated

3 years ago
blocking-b2g: 2.0M? → 2.0M+
Please nominate this for b2g34 and b2g37 when you get a chance.
status-firefox38: --- → wontfix
status-firefox39: --- → wontfix
Flags: needinfo?(hchang)
(Assignee)

Comment 15

3 years ago
Comment on attachment 8582148 [details] [diff] [review]
Bug1146248.patch

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): DHCP
User impact if declined: Quickly run dhcp multiple times would fail to get corrent DHCP info
Testing completed: Yes
Risk to taking this patch (and alternatives if risky): No
String or UUID changes made by this patch:No
Flags: needinfo?(hchang)
Attachment #8582148 - Flags: approval-mozilla-b2g37?
Attachment #8582148 - Flags: approval-mozilla-b2g34?

Updated

3 years ago
Attachment #8582148 - Flags: approval-mozilla-b2g37?
Attachment #8582148 - Flags: approval-mozilla-b2g37+
Attachment #8582148 - Flags: approval-mozilla-b2g34?
Attachment #8582148 - Flags: approval-mozilla-b2g34+
You need to log in before you can comment on or make changes to this bug.