Closed Bug 97605 Opened 23 years ago Closed 22 years ago

PAC: PAC execution should not block the UI thread

Categories

(Core :: Networking, defect)

defect
Not set
major

Tracking

()

VERIFIED INVALID
Future

People

(Reporter: tingley, Assigned: darin.moz)

References

()

Details

(Keywords: arch)

Spun off from bug 93924. PAC execution occurs synchronously in the UI thread, so that appropriate channels may be opened based on the results. Unfortunately, PACs sometimes make synchronous calls to nsDNSService::Resolve -- any PAC containing dnsResolve(), isInNet(), or isResolvable() may make these calls. Synchronous calls to nsDNSService::Resolve() will ignore the DNS cache (bug 97097), but adding a one-element cache directly in PAC (bug 93924) will improve this somewhat. The real issue is in the case where the name is not resolvable, in which case the DNS service may take a while to time-out. Until it does, the UI thread is blocked, effectively hanging the browser. I've discussed the problem with bbaetz a bit, and there doesn't seem to be an easy solution.
+arch
Severity: normal → major
Keywords: arch
QA Contact: benc → pacqa
Its not that this is hard, its that it cannot be fixed, I think. a) necko isn't threadsafe, so PAC will always be called on the UI thread b) We need PAC to be synchronous, because we a caller which calls NewChannelFromURI needs to be able to QI it to an http chanel immediately if we are using an http proxy. Thus, PAC must be run synchronously on the UI channel. (OK, we could use a wrapper class which then forwards to the real channel once its known, after calling pac asynchronously, when a method is called, _or_ when a QI is made. I don't see the benefits being worth the complexity, however. Even then it probably wouldn't help, because calling AsyncOpen after NewChannel isn't that uncommon. We can't proxy the AsyncOpen as well, because it needs to return an error) darin?
actually, perhaps a much simpler solution would be to push an event queue, which would still allow UI events, but the PAC download would then become an app modal operation. dougt: does this sound possible?
It would be easy enough to try.
Blocks: 79893
A similar problem will probably be caused by a fix for bug 80918, unless "No proxy for" on IP addresses continues to not resolve the hostname before doing a comparison.
This bug is unfortunately invalid. PAC needs to block the ui thread, because consumers creating a channel need to be able to QI it at once to an nsIHttpChannel if appropriate. Creating a channel is not an async operation; we can't run this off a separate event. Once bug 97097 is fixed (currently targetted for 0.9.6), then we shouldn't have any problems with speed, I think. darin, does that sound right?
Depends on: 97097
-> future
Assignee: neeti → darin
Target Milestone: --- → Future
bradley: should this be RESOLVED/INVALID then?
Unfortunately, yes
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → INVALID
VERIFIED (byebye) Is this any worse than having a manual config that points to a proxy hostname that is unresolvable?
Status: RESOLVED → VERIFIED
Blocks: 235853
You need to log in before you can comment on or make changes to this bug.