97097 - nsIDNSService::Resolve does not use the dns cache

Status: VERIFIED FIXED

(Core :: Networking: Cache, defect, P1)

Description

[Bradley Baetz (:bbaetz)]

The synchronous dns resolve method doesn't use the DNS cache. This is bad
because this function is used often in the PAC code, which must do a synchronous
lookup. The same host is looked up more than once in lots of pac files for
functions like isInNet, and I think that everyone assumed that the dns cache
would get this.

(Although I am going to suggest that the pac code does a one-entry cache, like
the mozillaClassic code did, to avoid unneeded xpconnect wrapping.

Comment 1

[gordon]

This seems doable in the 0.9.6 timeframe.

Comment 2

[Ashley Bischoff (blog at handcoding.com)]

*** Bug 104819 has been marked as a duplicate of this bug. ***

Comment 3

[gordon]

Moving to necko performance milestone 0.9.7.

Comment 4

[Vinay Badami]

Attachment: attaching patch

Please review the attached patch and let me know if there are problems.

Comment 5

[Vinay Badami]

Attachment: Protect table lookup with lock

Darin pointed out that the hash table lookup needs to be protected by the dns
lock.
Doing so.

Comment 6

[gordon]

Attachment: updated patch to protect access of lookup's buffer. Removed unnecessary Addref & Release.

Comment 7

[gordon]

Gagan, Darin, I just submitted a new patch for your [super-]review.  Is there a
good stress test for Resolve()?

Comment 8

[gordon]

Gagan, Darin, please review.

Comment 9

[gordon]

Attachment: replaces inet_ntoa() whose thread-safety is somewhat dubious.

Comment 10

[gordon]

Comment on attachment 70212 [details] [diff] [review]
replaces inet_ntoa() whose thread-safety is somewhat dubious.

sr=darin

Comment 11

[Gagan]

Comment on attachment 70212 [details] [diff] [review]
replaces inet_ntoa() whose thread-safety is somewhat dubious.

r=gagan

Comment 12

[gordon]

Patch checked in.  Marking FIXED.

Comment 13

[Tom Everingham]

verified