Closed
Bug 540088
Opened 16 years ago
Closed 16 years ago
GCAutoEnter needs to save and restore GCHeap->activeGC
Categories
(Tamarin Graveyard :: Garbage Collection (mmGC), defect)
Tamarin Graveyard
Garbage Collection (mmGC)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: stejohns, Unassigned)
Details
Attachments
(2 files)
|
2.26 KB,
patch
|
treilly
:
review+
lhansen
:
superreview-
|
Details | Diff | Splinter Review |
|
2.29 KB,
patch
|
treilly
:
review+
|
Details | Diff | Splinter Review |
If you have two nested GCAutoEnters, each using a separate GC instance, the dtor of the inner one will clear GCHeap->activeGC, causing improper behavior of things that rely on it.
|
Reporter | |
Updated•16 years ago
|
Attachment #421945 -
Attachment is patch: true
Attachment #421945 -
Attachment mime type: application/octet-stream → text/plain
Attachment #421945 -
Flags: superreview?(lhansen)
Attachment #421945 -
Flags: review?(treilly)
|
|
||
Comment 1•16 years ago
|
||
Comment on attachment 421945 [details] [diff] [review]
Patch
So far MMgc has avoided the antipattern of underscore-prefixed member names so sr- for that. If we feel a prefix is needed, "m_" is it.
As far as the functionality is concerned, I'm deferring to Tommy. But looking at the state of the code we probably want more design doc'n for this class. But that's just btw, it's not the job of this patch to provide it.
Attachment #421945 -
Flags: superreview?(lhansen) → superreview-
|
|
||
Comment 2•16 years ago
|
||
Comment on attachment 421945 [details] [diff] [review]
Patch
It makes me nervous to allow this so late but we know it happens and GC's nested properly before so I'm cool with it sans the underscores.
Attachment #421945 -
Flags: review?(treilly) → review+
|
|
Reporter | |
Comment 3•16 years ago
|
||
(In reply to comment #1)
> So far MMgc has avoided the antipattern of underscore-prefixed member names so
> sr- for that. If we feel a prefix is needed, "m_" is it.
If I agree to s/_/m_/ does that constitute sr+?
> As far as the functionality is concerned, I'm deferring to Tommy. But looking
> at the state of the code we probably want more design doc'n for this class.
> But that's just btw, it's not the job of this patch to provide it.
Agreed and agreed.
|
|
||
Comment 4•16 years ago
|
||
(In reply to comment #3)
> (In reply to comment #1)
> > So far MMgc has avoided the antipattern of underscore-prefixed member names so
> > sr- for that. If we feel a prefix is needed, "m_" is it.
>
> If I agree to s/_/m_/ does that constitute sr+?
Yup.
|
|
Reporter | |
Comment 5•16 years ago
|
||
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
|
|
Reporter | |
Comment 6•16 years ago
|
||
The assertion added in the bug fix needs to be removed; it turns out that we explicitly need to allow nested GCAutoEnter in a few obscure situations.
Attachment #422243 -
Flags: review?(treilly)
|
|
||
Updated•16 years ago
|
Attachment #422243 -
Flags: review?(treilly) → review+
You need to log in
before you can comment on or make changes to this bug.
Description
•