Closed Bug 551998 Opened 14 years ago Closed 14 years ago

If the key in a weak-key Dictionary is a string, it will not get GC'd or removed from the dictionary

Categories

(Tamarin Graveyard :: Garbage Collection (mmGC), defect, P3)

defect

Tracking

(Not tracked)

VERIFIED INVALID
flash10.1

People

(Reporter: mike, Assigned: lhansen)

Details

Attachments

(1 file)

Attached file sample app
"new Dictionary(true)" can be used to create a dictionary with weak keys.  If you add a key of type Object, and there are no other references to that Object, it will get GC'd and removed from the Dictionary, as it should; but if you add a key of type String, it will not get GC'd or removed from the Dictionary.

Steps:
1. Download dictionary.as, compile and run.  It creates a Dictionary with weak keys, and adds two keys: a "new Object()", and a dynamically constructed String.  It then dumps the keys, then forces a GC, and then dumps the keys again.

Actual results:

	before gc:
		[object Object]
		!!!
	after gc:
		!!!

Expected results:

	before gc:
		[object Object]
		!!!
Attachment #432182 - Attachment mime type: application/applefile → text/plain
Expected results should say:

	before gc:
		[object Object]
		!!!
	after gc:
This impacts the profiler.  Flash Builder's profiler implementation keeps track of objects using a Dictionary with weak keys.  Because Strings will never get freed, they show up in the user's profile results as strings that are in use by his program, but then if the user asks to see the path to gc root, there are none (because in fact the only path to a gc root is through the profiler's own data structures, which are, of course, ignored when constructing paths to gc roots).
Assignee: nobody → lhansen
Flags: flashplayer-qrb+
Priority: -- → P3
Target Milestone: --- → flash10.1
Status: NEW → ASSIGNED
The profiler tie-in is probably a red herring.  At issue is the behavior of Dictionary.  If this is not an injection since 10.0 -- ie if string keys are not removed from the dictionary in 10.0 -- then a change may need versioning and be off the table for 10.1.
You're right, Lars -- the profiler's use of a Dictionary is an implementation detail, and if that implementation doesn't work, then the profiler should use a different one.

I'm actually starting to really question whether there is a bug here at all:

The documentation for Dictionary [1] comes right to the verge of directly addressing this. It says that it "uses strict equality (===) for key comparison on non-primitive object keys."  It goes on to say, "Primitive (built-in) objects, like Numbers, in a Dictionary collection behave in the same manner as they do when they are the property of a regular object."  

In this context, do Strings count as primitive, or non-primitive?  If they count as primitive, then the current behavior is correct.  But even if one felt that Strings should count as non-primitive, the ECMAScript spec [2] clearly states that strict equality of two Strings is done by comparing the characters, not the pointers: "return true if x and y are exactly the same sequence of characters (same length and same characters in corresponding positions); otherwise, return false."

Considering that, putting Strings into a Dictionary and treating them as non-primitive seems kind of useless.  Suppose the user's code does this:

    s = ... // some unique string
    d = new Dictionary(true);
    d[s] = true;

    s2 = ... // regenerate a new copy of that same string
    value = d[s2]; // this will work

Of course d[s2] will get the value stored by d[s], because strict equality of strings is done by comparing the characters, not the string pointers.  So... what difference does it really make if 's' has been nulled out?  Once you use a string as a key in a Dictionary, the original string is essentially irrelevant.  Therefore, maybe it doesn't make sense to remove entries from a Dictionary when the original string is gc'd.

I think the reason I started going down the wrong path on this is that I was spending a lot of time in the C++ code, where String is an object with a pointer.  Thinking about things in terms of ActionScript semantics, the current behavior may be correct.

[1] http://livedocs.adobe.com/flash/9.0/ActionScriptLangRefV3/flash/utils/Dictionary.html
[2] http://www.ecma-international.org/publications/standards/Ecma-262.htm
Strings are primitive, as are Namespace, Number, Boolean, null, and undefined.  Indeed it makes scant sense for strings and namespaces to be weakly held, and no sense at all for the other primitives.

With that in mind, the documentation for Dictionary is unambiguous and the bug is invalid.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → INVALID
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: