If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

nsDNSService doesn't shutdown

VERIFIED FIXED in mozilla0.9

Status

()

Core
Networking
--
critical
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: Shin'ichiro TAYA, Assigned: Darin Fisher)

Tracking

({verifyme})

Trunk
mozilla0.9
All
NetBSD
verifyme
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

17 years ago
nsDNSService creates threads at nsDNSRequest::Init().
Thread created, but the threads doesn't run until actual DNS lookup request
occured(_PR_RUNNABLE state).
regchrome does no DNS lookup, so thread never run(activate).
At shudown time,  nsDNSService::~nsDNSService() calls nsDNSService::Shutdown()
and nsDNSService::Shutdown() calls mThread->Join().
Joining to DNS lookup thread make the thread active.
So the thread tries to dequeue DNS lookup request from queue, but no request on
the queue, then the thread begin to wait DNS lookup request.
But the request will never happen, then dead lock occures.
Maybe this is a combination of NSPR thread and new sync DNS lookup code.
I don't have an idea to fix this problem, but regchrom is required to make
NetBSD package, it's a big problem for me (and NetBSD).
Without regchrome, user have to run mozilla as root once.

Comment 1

17 years ago
cc'ing networking guys and confirming bug (looks valid to me).
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 2

17 years ago
Created attachment 28017 [details] [diff] [review]
workaround for this issue
(Reporter)

Comment 3

17 years ago
I submitted a tiny workaround for this issue.
I don't known this is a right way.
Even if this is a right way, need to consider that examine shutdownInProgress at
more part(e.g. EnqueueLookup()) or not, and consider lock 'shutdownInprogress'
or not while accessing this variable.
Any way, with this patch, mozilla & regchrome works on NetBSD.
(Assignee)

Comment 4

17 years ago
Looks good to me!  Thanks for tracking this down.  r=darin

Comment 5

17 years ago
-->dns
Assignee: neeti → darin
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Keywords: nsbeta1
Target Milestone: --- → mozilla0.9
(Assignee)

Updated

17 years ago
Keywords: patch
(Assignee)

Comment 6

17 years ago
Created attachment 28590 [details] [diff] [review]
Cleaned up patch
(Reporter)

Comment 7

17 years ago
with the patch darin made, I could run regchrome.

Comment 8

17 years ago
A couple of things:

1.  The check for mShutdownInProgress should be in the if statement:

something like:

if (mLookupQ && !mShutdownInProgress) {

Is much better since you still get the nulling of |*lookup|.


2.  although not needed for this patch, I really think that this variable should
be cross platform. 


do (1) and r|sr=dougt
(Assignee)

Comment 9

17 years ago
dougt: thanks for the review!  in response to your comments...

1) yeah probably a good idea, athough not absolutely necessary given the way
   DequeueLookup is used.

2) this is only needed for XP_UNIX since the other platforms do not wait on a
   monitor for new lookup requests.

Comment 10

17 years ago
r=gordon
(Assignee)

Comment 11

17 years ago
fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Updated

17 years ago
QA Contact: tever → bbaetz
This code got removed with the dns cache stuff. I'm going to assume that people
would have shouted loudly by now if we stopped working on NetBSD, but I'm not
sure. I'll verify it if noone speaks up in the next few days.
Keywords: verifyme

Comment 13

17 years ago
regxpcom still works fine now.
Maybe DNS_SHUTTING_DOWN state handle this issue correctly now.
then VERIFIED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.