Closed Bug 1146248 Opened 5 years ago Closed 5 years ago

Avoid concurrent dhcp_do_request execution

Categories

(Firefox OS Graveyard :: Wifi, defect)

x86
Gonk (Firefox OS)
defect
Not set

Tracking

(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)

RESOLVED FIXED
2.2 S9 (3apr)
blocking-b2g 2.0M+
Tracking Status
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

People

(Reporter: hchang, Assigned: hchang)

References

Details

Attachments

(2 files)

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: nobody → hchang
Attached patch Bug1146248.patchSplinter Review
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)
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+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0ac8ff6c3bed
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S9 (3apr)
Duplicate of this bug: 1133159
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?
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+
blocking-b2g: 2.0M? → 2.0M+
Please nominate this for b2g34 and b2g37 when you get a chance.
Flags: needinfo?(hchang)
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?
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.