Closed Bug 1839147 Opened 1 year ago Closed 1 year ago

PHC can get stuck in disabled state

Categories

(Core :: Memory Allocator, defect, P1)

defect

Tracking

()

RESOLVED FIXED
116 Branch
Tracking Status
firefox116 --- fixed

People

(Reporter: pbone, Assigned: pbone)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

PHC can get stuck with PHC disabled for a long time if the following occurs:

  • sAllocDelay is very low but not zero - that is, it's almost time do do a PHC allocation.
  • PHC is disabled either by API or itself to protect against reenterant use.
  • One or more allocations pushes sAllocDelay "below zero", but the zero condition is never detected because PHC is disabled on this thread.
  • PHC is enabled, but now sAllocDelay == 0 never happens, at least not until 2^32 allocations happens first so PHC won't be used for new allocations for a long time.

Note that sAllocDelay is unsigned, so when I say "below zero" I mean figuratively. This is in contradiction to the comment here: https://searchfox.org/mozilla-central/source/memory/replace/phc/PHC.cpp#1042-1044

This doesn't seem to be used by Firefox explicitly, at least I didn't find any:

https://searchfox.org/mozilla-central/search?q=symbol:_ZN13ReplaceMalloc26ReenablePHCOnCurrentThreadEv&redirect=false

PHC does do this itself to avoid reenterant situations. So this could occur in free() or realloc() if they do some allocation while capturing a stack. It won't happen for malloc() since it resets the sAllocDelay counter before returning.

Depends on D181422

Depends on D181831

Pushed by pbone@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/78a94e09f63c Fix PHC getting stuck in disabled state r=glandium https://hg.mozilla.org/integration/autoland/rev/8f25834121a8 Fix comments about sAllocDelay's type r=glandium https://hg.mozilla.org/integration/autoland/rev/f25864809591 Fix linter warnings after rearranging code r=glandium
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 116 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: