Replace NS_TLS/TlsAlloc with mozilla::ThreadLocal

NEW
Unassigned

Status

()

6 years ago
6 years ago

People

(Reporter: glandium, Unassigned)

Tracking

Trunk
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Reporter)

Updated

6 years ago
No longer depends on: 753119
Depends on: 753119
(Reporter)

Updated

6 years ago
Depends on: 756965
(Reporter)

Comment 1

6 years ago
Created attachment 625565 [details] [diff] [review]
Replace NS_TLS/TlsAlloc with mozilla::ThreadLocal in xpcom

This doesn't actually work correctly, because NS_IsCycleCollectorThread is called[1] before ThreadManager::Init.

1. The call stack is:
#0  0x00007ffff7bcfefb in raise (sig=<optimized out>) at ../nptl/sysdeps/unix/sysv/linux/pt-raise.c:41
#1  0x00007ffff2ef4435 in mozilla::ThreadLocal<mozilla::threads::ID>::get (this=0x7ffff52fab10) at ../../dist/include/mozilla/ThreadLocal.h:113
#2  0x00007ffff3d42dbb in NS_IsCycleCollectorThread_P () at ../../dist/include/nsCycleCollectorUtils.h:51
#3  nsDirEnumeratorUnix::AddRef (this=0x7ffff6b1e8c0) at /home/mh/mozilla-central/xpcom/io/nsLocalFileUnix.cpp:181
#4  0x00007ffff3d42280 in nsLocalFile::GetDirectoryEntries (this=this@entry=0x7ffff6b273c0, entries=entries@entry=0x7fffffffbe50) at /home/mh/mozilla-central/xpcom/io/nsLocalFileUnix.cpp:1708
#5  0x00007ffff2f00341 in nsXREDirProvider::LoadAppBundleDirs (this=this@entry=0x7fffffffc0b8) at /home/mh/mozilla-central/toolkit/xre/nsXREDirProvider.cpp:590
#6  0x00007ffff2f00652 in nsXREDirProvider::Initialize (this=0x7fffffffc0b8, aXULAppDir=0x7ffff6b27480, aGREDir=<optimized out>, aAppProvider=<optimized out>) at /home/mh/mozilla-central/toolkit/xre/nsXREDirProvider.cpp:147
#7  0x00007ffff2ef8c52 in XREMain::XRE_mainInit (this=this@entry=0x7fffffffc078, aAppData=aAppData@entry=0x61b5b0, aExitFlag=aExitFlag@entry=0x7fffffffc01f) at /home/mh/mozilla-central/toolkit/xre/nsAppRunner.cpp:2968

which also means the current windows implementation, using TlsAlloc/TlsGetValue, is fragile: gTLSThreadIDIndex is 0 before ThreadManager::Init. If something else already allocated a TLS key, TlsGetValue(0) may return an unrelated value.

I'm tempted to say this is one of the rare cases where a static initializer would be better.
(Reporter)

Comment 2

6 years ago
Created attachment 625609 [details] [diff] [review]
Replace NS_TLS/TlsAlloc with mozilla::ThreadLocal in xpcom

This variant works on debug builds, except windows debug.
(Reporter)

Updated

6 years ago
Attachment #625565 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.