Changeset 66665 in webkit


Ignore:
Timestamp:
Sep 2, 2010 5:57:58 AM (14 years ago)
Author:
ap@apple.com
Message:

2010-09-02 Alexey Proskuryakov <​ap@apple.com>

Reviewed by Oliver Hunt.

​https://bugs.webkit.org/show_bug.cgi?id=43230
<rdar://problem/8254215> REGRESSION: Memory leak within JSParser::JSParser

One can't delete a ThreadSpecific object that has data in it. It's not even possible to
enumerate data objects in all threads, much less destroy them from a thread that's destroying
the ThreadSpecific.

  • parser/JSParser.cpp: (JSC::JSParser::JSParser):
  • runtime/JSGlobalData.h:
  • wtf/WTFThreadData.cpp: (WTF::WTFThreadData::WTFThreadData):
  • wtf/WTFThreadData.h: (WTF::WTFThreadData::approximatedStackStart): Moved stack guard tracking from JSGlobalData to WTFThreadData.
  • wtf/ThreadSpecific.h: Made destructor unimplemented. It's dangerous, and we probably won't ever face a situation where we'd want to delete a ThreadSpecific object.
Location:
trunk/JavaScriptCore
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/JavaScriptCore/ChangeLog

    r66616 r66665  
     12010-09-02  Alexey Proskuryakov  <ap@apple.com>
     2
     3        Reviewed by Oliver Hunt.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=43230
     6        <rdar://problem/8254215> REGRESSION: Memory leak within JSParser::JSParser
     7
     8        One can't delete a ThreadSpecific object that has data in it. It's not even possible to
     9        enumerate data objects in all threads, much less destroy them from a thread that's destroying
     10        the ThreadSpecific.
     11
     12        * parser/JSParser.cpp:
     13        (JSC::JSParser::JSParser):
     14        * runtime/JSGlobalData.h:
     15        * wtf/WTFThreadData.cpp:
     16        (WTF::WTFThreadData::WTFThreadData):
     17        * wtf/WTFThreadData.h:
     18        (WTF::WTFThreadData::approximatedStackStart):
     19        Moved stack guard tracking from JSGlobalData to WTFThreadData.
     20
     21        * wtf/ThreadSpecific.h: Made destructor unimplemented. It's dangerous, and we probably won't
     22        ever face a situation where we'd want to delete a ThreadSpecific object.
     23
    1242010-09-01  Gavin Barraclough  <barraclough@apple.com>
    225

  • trunk/JavaScriptCore/parser/JSParser.cpp

    r65104 r66665  
    3434#include "ASTBuilder.h"
    3535#include <wtf/HashFunctions.h>
     36#include <wtf/WTFThreadData.h>
    3637#include <utility>
    3738
    … …  
    221222    , m_syntaxAlreadyValidated(provider->isValid())
    222223{
    223     m_endAddress = *(globalData->stackGuards);
    224     if (!m_endAddress) {
    225         char sample = 0;
    226         m_endAddress = &sample - kMaxParserStackUsage;
    227         *(globalData->stackGuards) = m_endAddress;
    228     }
     224    m_endAddress = wtfThreadData().approximatedStackStart() - kMaxParserStackUsage;
    229225    next();
    230226    m_lexer->setLastLineNumber(tokenLine());

  • trunk/JavaScriptCore/runtime/JSGlobalData.h

    r65947 r66665  
    228228
    229229        CachedTranscendentalFunction<sin> cachedSin;
    230         WTF::ThreadSpecific<char*> stackGuards;
    231230
    232231        void resetDateCache();

  • trunk/JavaScriptCore/wtf/ThreadSpecific.h

    r61732 r66665  
    6868    operator T*();
    6969    T& operator*();
    70     ~ThreadSpecific();
    7170
    7271private:
    … …  
    7473    friend void ThreadSpecificThreadExit();
    7574#endif
     75
     76    // Not implemented. It's technically possible to destroy a thread specific key, but one would need
     77    // to make sure that all values have been destroyed already (usually, that all threads that used it
     78    // have exited). It's unlikely that any user of this call will be in that situation - and having
     79    // a destructor defined can be confusing, given that it has such strong pre-requisites to work correctly.
     80    ~ThreadSpecific();
    7681   
    7782    T* get();
    … …  
    117122
    118123template<typename T>
    119 inline ThreadSpecific<T>::~ThreadSpecific()
    120 {
    121 }
    122 
    123 template<typename T>
    124124inline T* ThreadSpecific<T>::get()
    125125{
    … …  
    144144
    145145template<typename T>
    146 inline ThreadSpecific<T>::~ThreadSpecific()
    147 {
    148     pthread_key_delete(m_key); // Does not invoke destructor functions.
    149 }
    150 
    151 template<typename T>
    152146inline T* ThreadSpecific<T>::get()
    153147{
    … …  
    171165
    172166template<typename T>
    173 inline ThreadSpecific<T>::~ThreadSpecific()
    174 {
    175     // Does not invoke destructor functions. QThreadStorage will do it
    176 }
    177 
    178 template<typename T>
    179167inline T* ThreadSpecific<T>::get()
    180168{
    … …  
    197185{
    198186    g_static_private_init(&m_key);
    199 }
    200 
    201 template<typename T>
    202 inline ThreadSpecific<T>::~ThreadSpecific()
    203 {
    204     g_static_private_free(&m_key);
    205187}
    206188

  • trunk/JavaScriptCore/wtf/WTFThreadData.cpp

    r65021 r66665  
    4444#endif
    4545{
     46    char sample = 0;
     47    m_approximatedStackStart = &sample;
    4648}
    4749

  • trunk/JavaScriptCore/wtf/WTFThreadData.h

    r65021 r66665  
    113113#endif
    114114
     115    char* approximatedStackStart() const
     116    {
     117        return m_approximatedStackStart;
     118    }
     119
    115120private:
    116121    AtomicStringTable* m_atomicStringTable;
    … …  
    129134    friend WTFThreadData& wtfThreadData();
    130135    friend class AtomicStringTable;
     136
     137    char* m_approximatedStackStart;
    131138};
    132139
Note: See TracChangeset for help on using the changeset viewer.