Closed Bug 1463961 Opened 3 years ago Closed 3 years ago

DllInterceptor shouldn't malloc and init locks in static initializers


(Core :: XPCOM, enhancement)

Not set



Tracking Status
firefox62 --- fixed


(Reporter: dmajor, Assigned: aklotz)




(1 file)

Blocks: 1432653
We can get rid of the allocation and mapping easily enough; that stuff was really only there for sharing trampoline space with cross-process interceptors, which I have decided that I don't want to support anyway. It also gives us the added bonus of removing an extra level of indirection.

As for the CS, that one is harder to steer around. I'd switch it to SRWLOCKs if it wasn't for the fact that we actually depend on recursive entry.

I think the real question at hand is, is there any good reason to statically initialize those blocklist interceptors in the first place? In general our coding style guide bans static initializers, so while I'm happy to clean up some of this code for the other benefits that it brings, I'm more inclined to suggest that, if we're concerned enough about the static initializers in mozglue, we should just switch those interceptor instantiation sites to use dynamic allocation.
Assignee: nobody → aklotz
Comment on attachment 8981280 [details] [diff] [review]
Make shared VM policy only work with in-proc memory policies and remove pid mapping

Review of attachment 8981280 [details] [diff] [review]:

I'd nitpick the use of the magic number 4000 in the InitializeCriticalSectionEx call but we do it in a number of places and its not hard to find in the docs so why bother.

I'm a bit weirded out by all of the static WindowsDllInterceptors we have.  But I think I agree that finding a proper place to early-init them would be more trouble than its worth.
Attachment #8981280 - Flags: review?(davidp99) → review+
Summary: DllInterceptor shouldn't malloc and take locks in static initializers → DllInterceptor shouldn't malloc and init locks in static initializers
Bug 1463961: DLL Interceptor - Make shared VM policy only work for in-proc interceptors and remove pid mapping; r=handyman
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.