594422 - ByteArray addSubscriber broken

Status: RESOLVED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P2)

Description

[Tommy Reilly]

Write barrier missing in weakSubscriber write, this code should be re-written to use smart pointer write barriers as well.

Comment 1

[Steven Johnson]

Is this in the avmshell or Flash ByteArray? (or both?)

Comment 2

[Tommy Reilly]

both (WE bug: 2714615)

Comment 3

[Tommy Reilly]

Attachment: Use DWB's on all GC pointers

Comment 4

[Tommy Reilly]

http://hg.mozilla.org/tamarin-redux/rev/21bf8949750b

Comment 5

[Steven Johnson]

We should update this bug with the CL of the fix to Flash's parallel ByteArray implementation.

(See also https://bugzilla.mozilla.org/show_bug.cgi?id=564248, which also has ByteArray update notes)

Comment 6

[Steven Johnson]

Note, this happens to work OK in Tamarin, but making m_subscriberRoot a DWB won't work in the Flash/AIR version of this file: their ByteArray is used on the stack (as well as being embedded in a GC object), where DWB is verboten.

Q: instead of a linked list, how about we use a List<> of GCWeakRef? It's going to be a pretty small list no matter what, and List<> is already savvy about working in both stack and gc-member contexts...

Comment 7

[Lars T Hansen]

(In reply to comment #6)
> Note, this happens to work OK in Tamarin, but making m_subscriberRoot a DWB
> won't work in the Flash/AIR version of this file: their ByteArray is used on
> the stack (as well as being embedded in a GC object), where DWB is verboten.

For exact tracing, objects are going to be required to be heap-allocated, and there will be asserts that check this.  GCObjects (etc) that are members or stack-allocated can no longer be that.

Comment 8

[Tommy Reilly]

(In reply to comment #6)
> Note, this happens to work OK in Tamarin, but making m_subscriberRoot a DWB
> won't work in the Flash/AIR version of this file: their ByteArray is used on
> the stack (as well as being embedded in a GC object), where DWB is verboten.

I'll have a go at fixing it.

Comment 9

[Steven Johnson]

(In reply to comment #7)
> For exact tracing, objects are going to be required to be heap-allocated, and
> there will be asserts that check this.  GCObjects (etc) that are members or
> stack-allocated can no longer be that.

ByteArray is not a GCObject, it's just a plain C++ object that contains a pointer-to-GCObject. See existing List<> class for a similar precedent.