Closed Bug 283234 Opened 19 years ago Closed 19 years ago

jsarena.c arena reuse + reallocation can lose other allocations.

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.8beta2

People

(Reporter: dylan, Assigned: brendan)

Details

(Keywords: js1.5)

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0

In JS_ArenaAllocate(), if the allocation is oversized, the function may select
an arena from the free list that is larger than necessary, as noted in this comment:

/*
 * Insist on exact arenasize match if nb is not greater than
 * arenasize.  Otherwise take any arena big enough, but not by
 * more than gross + arenasize.
 */

But this leaves room for more (small) allocations to be added after this
allocation in the same arena, which violates the “every oversized allocation is
the only allocation in its arena” rule.  When the oversized allocation is
subsequently grown, other allocations will be lost.

This bug doesn’t get hit when all oversized allocations are powers of two, but
the Sprinter code does seem to be something that can create unusually-sized
allocations.

One way to fix this would be to push avail all the way out to limit for any
arena with an oversized allocation, and change the grow/realloc functions to
simply leave the arena alone if the caller wants to grow the allocation into
space that the arena already has.


Reproducible: Always

Steps to Reproduce:
Comments in this file show where the smaller allocation will get lost.
This could explain talkback from crashes to-do with NewOrRecycledNode and other
arena-based allocators.  I'd like to fix it ASAP for impending releases.

/be
Flags: blocking1.7.6?
Flags: blocking-aviary1.0.1?
Keywords: js1.5
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta2
Assignee: general → brendan
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch simplest fixSplinter Review
I don't see the point in messing around here.

/be
Attachment #175232 - Flags: review?(shaver)
Status: NEW → ASSIGNED
Don't you need to fix the comment right above that?
Jband, yeah: I was rushing to get into Firefox 1.0.1, but that train has left.
Here's the revised comment I'll check into the trunk:

                 /*
-                 * Insist on exact arenasize match if nb is not greater than
-                 * arenasize.  Otherwise take any arena big enough, but not by
-                 * more than gross + arenasize.
+                 * Insist on exact arenasize match to avoid leaving alloc'able
+                 * space after an oversized allocation as it grows.
                  */

/be
Comment on attachment 175232 [details] [diff] [review]
simplest fix

r=shaver, thanks for the IRC explanations as well.
Attachment #175232 - Flags: review?(shaver) → review+
Fixed on trunk.  Candidate for future dot releases off branches, but not gonna
hold up 1.0.1 (therefore ditto 1.7.6).

/be
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Flags: blocking1.7.6?
Flags: blocking-aviary1.0.1?
Resolution: --- → FIXED
Flags: testcase-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: