1360236 - hide gSocketThread's use in asserts and conditions behind a functional interface

Status: RESOLVED FIXED

(Core :: Networking, enhancement)

Description

[Nathan Froyd [:froydnj]]

As part of Quantum DOM, we're planning on curtailing the use of PR_GetCurrentThread to a few whitelisted places (details to be posted on dev-platform soonish).  The various assertions netwerk/ has on gSocketThread are a big piece of the current uses of PR_GetCurrentThread.

There are two options for reducing this:

1) Add bool OnSocketThread() to nsSocketTransportService2.{h,cpp} and replace all the current ==/!= tests, whether in assertions or otherwise, against gSocketThread with calls to that (out-of-line) function.

2) As in #1, but replace the MOZ_ASSERT(OnSocketThread()) calls with out-of-line AssertOnSocketThread() functions; we'd also need a corresponding AssertNotOnSocketThread for a few places.

In both scenarios, gSocketThread would become a private implementation detail of nsSocketTransportService2.cpp.

I was initially going to do #2, but I suspect that #1 is easier to implement and possibly a little more palatable.  Do you have a preference?

Comment 1

[Patrick McManus [:mcmanus]]

Thanks nathan. I love those asserts btw - super useful documentation and enforcement. So I appreciate you not just wanting to delete them :)

both your approaches are ok by me; I guess I have a  slight preference for #1.

Another option might be a gecko-wide functon AssertCurrentThread(thread *) that has a nop inline for NDEBUG and does the obvious out of line thing for DEBUG.

Comment 2

[Nathan Froyd [:froydnj]]

Attachment: make gSocketThread an internal implementation detail of nsSocketTransportService

This change makes the code a little cleaner and reduces the number of
places we call PR_GetCurrentThread, which is important for Quantum DOM
scheduling work.

The conversion was largely automatic, via:

find netwerk/ -name \*.cpp | \
  xargs sed -i -e 's/MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread[^;]*/MOZ_ASSERT(OnSocketThread(), "not on socket thread")/'

and related invocations, with a few manual tweaks at the end.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3dc6e29a0128
make gSocketThread an internal implementation detail of nsSocketTransportService; r=mcmanus

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3dc6e29a0128