Comment 33 for bug 367531

Revision history for this message
In , Patrick McManus (mcmanus-ducksong) wrote :

Comment on attachment 619220
netwerk: Add offline mode to DNS service and STS. Add RESOLVE_OFFLINE flag for nsHostResolver.

Review of attachment 619220:
-----------------------------------------------------------------

r=mcmanus with the processpendingevnets() issue addressed. Thanks!

::: netwerk/base/src/nsSocketTransport2.cpp
@@ +1082,5 @@
>
> nsresult rv;
>
> + if (gIOService->IsOffline() &&
> + PR_IsNetAddrType(&mNetAddr, PR_IpAddrLoopback) == PR_FALSE)

nit - style is to check !foo instead of foo == false

::: netwerk/base/src/nsSocketTransportService2.cpp
@@ +726,5 @@
> +
> + // Final pass over the event queue. This makes sure that events posted by
> + // socket detach handlers get processed.
> + nsIThread *thread = NS_GetCurrentThread();
> + NS_ProcessPendingEvents(thread);

2 problems here - I think you only want this on the real shutdown, not on the offline case.. and more importantly you now spin the event loop while holding the lock (back down 1 step on the stack) - I was able to deadlock this patch pretty easily because of that. but to fix it I think you just need to only spin it manually on shutdown after the lock is given up, like before.