profiler, a static member variable in GCHeap is not reset correctly with a call to GCHeap::DestroyInstance

VERIFIED FIXED in flash10.1

Status

Tamarin
Garbage Collection (mmGC)
P2
normal
VERIFIED FIXED
8 years ago
7 years ago

People

(Reporter: Kaiwalya Kher, Assigned: Tommy Reilly)

Tracking

unspecified
flash10.1
Other
Other
Bug Flags:
in-testsuite -
flashplayer-qrb +

Details

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3
Build Identifier: 

The variable is initialized in GCHeap.cpp:

MemoryProfiler* GCHeap::profiler = (MemoryProfiler*)-1;

In GCHeap::DestroyInstance it is reset as:
profiler = NULL;

Which means the function IsProfilerInitialized() which checks for (profiler != -1) will return the wrong result the next time.

Reproducible: Always

Steps to Reproduce:
1. Create GCHeap
2. Make allocations
3. Destroy GCHeap (Notice leak call stack traces are dumped)
4. Create GCHeap
5. Make allocations
6. Destroy GCHeap (Notice leak call stack traces are NOT dumped) 

Actual Results:  
No memory leaks on step 6 above

Expected Results:  
Memory leak call stack traces should be dumped irrespective of how many times the GCHeap has been initialized or destroyed.

profiler should be reset to "-1" inplace of "NULL" in GCHeap::DestroyInstance
(Reporter)

Comment 1

8 years ago
Created attachment 443410 [details] [diff] [review]
reset profiler to (MemoryProfiler *)-1 in DestroyInstance

Fix
Attachment #443410 - Flags: review?(treilly)
(Assignee)

Updated

8 years ago
Attachment #443410 - Flags: superreview?(lhansen)
Attachment #443410 - Flags: review?(treilly)
Attachment #443410 - Flags: review+

Comment 2

8 years ago
Comment on attachment 443410 [details] [diff] [review]
reset profiler to (MemoryProfiler *)-1 in DestroyInstance

As a general cleanup item we need to factor initialization of this kind into a separate method that can be called from both setup and takedown - probably.  It would reduce the risk of further error.

I'll create a bug for that work item.
Attachment #443410 - Flags: superreview?(lhansen) → superreview+

Updated

8 years ago
Assignee: nobody → treilly
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P2
Target Milestone: --- → flash10.1
(Reporter)

Updated

8 years ago
Keywords: checkin-needed

Updated

8 years ago
Flags: flashplayer-qrb+
(Assignee)

Comment 3

8 years ago
tamarin-redux-argo:
changeset:   4029:0da8f3b4ce1f
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Comment 4

8 years ago
tamarin-redux - changeset - 4658:0da8f3b4ce1f

Updated

8 years ago
Flags: in-testsuite?
QA Contact: gc → dschaffe

Comment 5

8 years ago
Lars:  I was trying to write a mmgc testcase, looking something like code below.  when I run the heap->Destroy() shell seg faults,  any suggestions on the proper way to destroy the heap?  how could I verify call stack traces are dumped?  

add to extensions/ST_mmgc_gcheap.st:

%%test DestroyInstance
       GCHeap *heap;
       heap = GCHeap::GetGCHeap();
       for(int i=1;i<10;i++) {
       	   void *item = heap->Alloc(GCHeap::kOSAllocThreshold*i, GCHeap::flags_Alloc, 1<<i);
       }
       heap->Destroy();
       // seg fault here

       heap = GCHeap::GetGCHeap();
       heap.Init(GCHeap::GCHeapConfig());
       for(int i=1;i<10;i++) {
       	   void *item = heap->Alloc(GCHeap::kOSAllocThreshold*i, GCHeap::flags_Alloc, 1<<i);
       }
       heap->Destroy();

       %%verify true == true  // not sure how to create an assert here

Comment 6

8 years ago
You can't do that - GCHeap creation and destruction are handled outside selftest.

Comment 7

8 years ago
do not have a mechanism to regression test since the shell creates a GCHeap.  any suggestions?

marking as verified.
Status: RESOLVED → VERIFIED
Flags: in-testsuite? → in-testsuite-
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.