593431 - Thread safety hazards in Traits

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Verifier, defect)

Description

[Edwin Smith]

Each instance of Traits has a 1-bit wide commonBase field, which is just a flag used during Verifier::findCommonBase().  

With upcoming background compilation, that single bit is not threadsafe.  It isn't worth locking around it; findCommonBase() should be rewritten so that it doesn't mutate the Traits objects being analyzed.

Comment 1

[Edwin Smith]

m_supertype_cache and m_supertype_neg_cache are mutated by suptypeof(), so might not be threadsafe.

This fact might be useful for a two-finger walk in Verifier::findCommonBase(): If the type is a normal class (not an interface) and its inheritance depth is < MAX_PRIMARY_SUPERTYPE, then we can compute its inheritance depth without a tree walk, from m_supertype_offset.

Comment 2

[Simon Wilkinson]

(In reply to comment #0)
> Each instance of Traits has a 1-bit wide commonBase field, which is just a flag
> used during Verifier::findCommonBase().  
> 
> With upcoming background compilation, that single bit is not threadsafe.  It
> isn't worth locking around it; findCommonBase() should be rewritten so that it
> doesn't mutate the Traits objects being analyzed.

I've already included a thread-safe version of findCommonBase in the main patch of bug 600723.
See section 5, 'Thread-Safety' in the design notes on comment 3 (https://bugzilla.mozilla.org/show_bug.cgi?id=600723#c3)

Taking another look though, I might not be handling interfaces correctly. Will take a look and report back...

Comment 3

[Simon Wilkinson]

(In reply to comment #2)
> 
> Taking another look though, I might not be handling interfaces correctly. Will
> take a look and report back...

I wasn't doing this correctly.
Latest patch of bug 600723 contains a fix.

Comment 4

[Simon Wilkinson]

Attachment: TR rev 6090. Patch queue rev 280

Factored the thread-safe implementation of Verifier::findCommonBase from bug 600723 into this patch.

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Tamarin is a dead project now. Mass WONTFIX.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Tamarin isn't maintained anymore. WONTFIX remaining bugs.