From bd6c9225328b6042ff14dfddb28e2e1279ba0e46 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Thu, 27 Jan 2011 10:01:48 +0100 Subject: Make syncqt not complain about missing header macros. qsharedpointer_impl.h contains class definitions that are also mirrored in qsharedpointer.h (which is there for qdoc3), so we have a macro to stop processing. Task-number: QTBUG-16912 Reviewed-by: Olivier Goffart --- src/corelib/tools/qsharedpointer_impl.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h index 2404d2a..0337c1f 100644 --- a/src/corelib/tools/qsharedpointer_impl.h +++ b/src/corelib/tools/qsharedpointer_impl.h @@ -44,7 +44,17 @@ #ifndef QSHAREDPOINTER_H #error Do not include qsharedpointer_impl.h directly #endif + #if 0 +// These macros are duplicated here to make syncqt not complain a about +// this header, as we have a "qt_sync_stop_processing" below, which in turn +// is here because this file contains a template mess and duplicates the +// classes found in qsharedpointer.h +QT_BEGIN_HEADER +QT_BEGIN_NAMESPACE +QT_MODULE(Core) +QT_END_NAMESPACE +QT_END_HEADER #pragma qt_sync_stop_processing #endif -- cgit v0.12 From 2e72a8b19ea6c674fb4777860dac50faa5d387e6 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Thu, 27 Jan 2011 11:49:49 +0100 Subject: Restore Qt 4.6 behaviour: exec() always enters the event loop. In Qt 4.6 as well as 4.7's QCoreApplication and QEventLoop, calling exec() always enters the event loop, even if you had tried to quit()/exit() it before entering, with one exception (noted in the unit tests; this difference has been in Qt since at least Qt 4.2). Add unit tests to ensure all of the three classes have the same behaviour. Decide if we want to match the behaviours in Qt 4.8. Reviewed-by: Bradley T. Hughes --- src/corelib/thread/qthread.cpp | 5 +- .../auto/qcoreapplication/tst_qcoreapplication.cpp | 45 +++++ tests/auto/qeventloop/tst_qeventloop.cpp | 41 ++++- tests/auto/qthread/tst_qthread.cpp | 191 +++++++++++++++------ 4 files changed, 221 insertions(+), 61 deletions(-) diff --git a/src/corelib/thread/qthread.cpp b/src/corelib/thread/qthread.cpp index f368192..f4bfa5d 100644 --- a/src/corelib/thread/qthread.cpp +++ b/src/corelib/thread/qthread.cpp @@ -482,10 +482,7 @@ int QThread::exec() Q_D(QThread); QMutexLocker locker(&d->mutex); d->data->quitNow = false; - if (d->exited) { - d->exited = false; - return d->returnCode; - } + d->exited = false; locker.unlock(); QEventLoop eventLoop; diff --git a/tests/auto/qcoreapplication/tst_qcoreapplication.cpp b/tests/auto/qcoreapplication/tst_qcoreapplication.cpp index 95055d1..bc69461 100644 --- a/tests/auto/qcoreapplication/tst_qcoreapplication.cpp +++ b/tests/auto/qcoreapplication/tst_qcoreapplication.cpp @@ -59,6 +59,9 @@ private slots: void applicationPid(); void globalPostedEventsCount(); void processEventsAlwaysSendsPostedEvents(); + void reexec(); + void execAfterExit(); + void eventLoopExecAfterExit(); }; class EventSpy : public QObject @@ -524,5 +527,47 @@ void tst_QCoreApplication::processEventsAlwaysSendsPostedEvents() } while (t.elapsed() < 3000); } +void tst_QCoreApplication::reexec() +{ + int argc = 1; + char *argv[] = { "tst_qcoreapplication" }; + QCoreApplication app(argc, argv); + + // exec once + QMetaObject::invokeMethod(&app, "quit", Qt::QueuedConnection); + QCOMPARE(app.exec(), 0); + + // and again + QMetaObject::invokeMethod(&app, "quit", Qt::QueuedConnection); + QCOMPARE(app.exec(), 0); +} + +void tst_QCoreApplication::execAfterExit() +{ + int argc = 1; + char *argv[] = { "tst_qcoreapplication" }; + QCoreApplication app(argc, argv); + + app.exit(1); + QMetaObject::invokeMethod(&app, "quit", Qt::QueuedConnection); + QCOMPARE(app.exec(), 0); +} + +void tst_QCoreApplication::eventLoopExecAfterExit() +{ + int argc = 1; + char *argv[] = { "tst_qcoreapplication" }; + QCoreApplication app(argc, argv); + + // exec once and exit + QMetaObject::invokeMethod(&app, "quit", Qt::QueuedConnection); + QCOMPARE(app.exec(), 0); + + // and again, but this time using a QEventLoop + QEventLoop loop; + QMetaObject::invokeMethod(&loop, "quit", Qt::QueuedConnection); + QCOMPARE(loop.exec(), 0); +} + QTEST_APPLESS_MAIN(tst_QCoreApplication) #include "tst_qcoreapplication.moc" diff --git a/tests/auto/qeventloop/tst_qeventloop.cpp b/tests/auto/qeventloop/tst_qeventloop.cpp index 7af722f..6860f19 100644 --- a/tests/auto/qeventloop/tst_qeventloop.cpp +++ b/tests/auto/qeventloop/tst_qeventloop.cpp @@ -112,6 +112,10 @@ signals: public: QMutex mutex; QWaitCondition cond; + volatile int result1; + volatile int result2; + MultipleExecThread() : result1(0xdead), result2(0xbeef) {} + void run() { QMutexLocker locker(&mutex); @@ -124,13 +128,13 @@ public: connect(&timer, SIGNAL(timeout()), SLOT(quit()), Qt::DirectConnection); timer.setInterval(1000); timer.start(); - (void) exec(); + result1 = exec(); // this should return immediately, since exit() has been called cond.wakeOne(); cond.wait(&mutex); QEventLoop eventLoop; - (void) eventLoop.exec(); + result2 = eventLoop.exec(); } }; @@ -197,7 +201,9 @@ private slots: void symbianNestedActiveSchedulerLoop(); void processEvents(); void exec(); + void reexec(); void exit(); + void execAfterExit(); void wakeUp(); void quit(); void processEventsExcludeSocket(); @@ -398,7 +404,9 @@ void tst_QEventLoop::exec() } { - // calling exec() after exit()/quit() should return immediately + // calling QEventLoop::exec() after a thread loop has exit()ed should return immediately + // Note: this behaviour differs from QCoreApplication and QEventLoop + // see tst_QCoreApplication::eventLoopExecAfterExit, tst_QEventLoop::reexec MultipleExecThread thread; // start thread and wait for checkpoint @@ -411,6 +419,8 @@ void tst_QEventLoop::exec() thread.cond.wakeOne(); thread.cond.wait(&thread.mutex); QVERIFY(spy.count() > 0); + int v = thread.result1; + QCOMPARE(v, 0); // exec should return immediately spy.clear(); @@ -418,6 +428,8 @@ void tst_QEventLoop::exec() thread.mutex.unlock(); thread.wait(); QCOMPARE(spy.count(), 0); + v = thread.result2; + QCOMPARE(v, -1); } { @@ -462,9 +474,32 @@ void tst_QEventLoop::exec() #endif } +void tst_QEventLoop::reexec() +{ + QEventLoop loop; + + // exec once + QMetaObject::invokeMethod(&loop, "quit", Qt::QueuedConnection); + QCOMPARE(loop.exec(), 0); + + // and again + QMetaObject::invokeMethod(&loop, "quit", Qt::QueuedConnection); + QCOMPARE(loop.exec(), 0); +} + void tst_QEventLoop::exit() { DEPENDS_ON(exec()); } +void tst_QEventLoop::execAfterExit() +{ + QEventLoop loop; + EventLoopExiter obj(&loop); + + QMetaObject::invokeMethod(&obj, "exit", Qt::QueuedConnection); + loop.exit(1); + QCOMPARE(loop.exec(), 0); +} + void tst_QEventLoop::wakeUp() { EventLoopThread thread; diff --git a/tests/auto/qthread/tst_qthread.cpp b/tests/auto/qthread/tst_qthread.cpp index e6bf9ce..c7036e4 100644 --- a/tests/auto/qthread/tst_qthread.cpp +++ b/tests/auto/qthread/tst_qthread.cpp @@ -86,6 +86,7 @@ private slots: void start(); void terminate(); void quit(); + void execAfterQuit(); void wait(); void started(); void finished(); @@ -265,6 +266,34 @@ public: } }; +class ExecAfterQuitThreadHelper: public QObject +{ + Q_OBJECT + QThread *thr; +public: + ExecAfterQuitThreadHelper(QThread *thr) : thr(thr) {} +public slots: + void doIt() { thr->exit(0); } +}; + +class ExecAfterQuitThread: public QThread +{ +public: + int returnValue; + void run() + { + ExecAfterQuitThreadHelper obj(this); + + QMetaObject::invokeMethod(&obj, "doIt", Qt::QueuedConnection); + exit(1); + + // returnValue will be either 0 or 1, depending on which of the two + // above take effect. The correct value is 0, since exit(1) before + // exec() should have no effect + returnValue = exec(); + } +}; + tst_QThread::tst_QThread() { @@ -424,34 +453,52 @@ void tst_QThread::stackSize() void tst_QThread::exit() { - Exit_Thread thread; - thread.object = new Exit_Object; - thread.object->moveToThread(&thread); - thread.code = 42; - thread.result = 0; - QVERIFY(!thread.isFinished()); - QVERIFY(!thread.isRunning()); - QMutexLocker locker(&thread.mutex); - thread.start(); - QVERIFY(thread.isRunning()); - QVERIFY(!thread.isFinished()); - thread.cond.wait(locker.mutex()); - QVERIFY(thread.wait(five_minutes)); - QVERIFY(thread.isFinished()); - QVERIFY(!thread.isRunning()); - QCOMPARE(thread.result, thread.code); - delete thread.object; + { + Exit_Thread thread; + thread.object = new Exit_Object; + thread.object->moveToThread(&thread); + thread.code = 42; + thread.result = 0; + QVERIFY(!thread.isFinished()); + QVERIFY(!thread.isRunning()); - Exit_Thread thread2; - thread2.object = 0; - thread2.code = 53; - thread2.result = 0; - QMutexLocker locker2(&thread2.mutex); - thread2.start(); - thread2.exit(thread2.code); - thread2.cond.wait(locker2.mutex()); - QVERIFY(thread2.wait(five_minutes)); - QCOMPARE(thread2.result, thread2.code); + QMutexLocker locker(&thread.mutex); + thread.start(); + QVERIFY(thread.isRunning()); + QVERIFY(!thread.isFinished()); + // but the thread is not running the event loop yet (the mutex is locked) + + // start the event loop + thread.cond.wait(locker.mutex()); + + // the Exit_Object above will cause the thread to exit + QVERIFY(thread.wait(five_minutes)); + QVERIFY(thread.isFinished()); + QVERIFY(!thread.isRunning()); + QCOMPARE(thread.result, thread.code); + delete thread.object; + } + + { + Exit_Thread thread2; + thread2.object = 0; + thread2.code = 53; + thread2.result = 0; + QMutexLocker locker2(&thread2.mutex); + thread2.start(); + + // the mutex is locked, so the thread has *not* started running the event loop yet + // this will do nothing: + thread2.exit(thread2.code); + + // the thread will now start running + thread2.cond.wait(locker2.mutex()); + + // this will cause it to exit now + thread2.exit(++thread2.code); + QVERIFY(thread2.wait(five_minutes)); + QCOMPARE(thread2.result, thread2.code); + } } void tst_QThread::start() @@ -498,32 +545,59 @@ void tst_QThread::terminate() void tst_QThread::quit() { - Quit_Thread thread; - thread.object = new Quit_Object; - thread.object->moveToThread(&thread); - thread.result = -1; - QVERIFY(!thread.isFinished()); - QVERIFY(!thread.isRunning()); - QMutexLocker locker(&thread.mutex); - thread.start(); - QVERIFY(thread.isRunning()); - QVERIFY(!thread.isFinished()); - thread.cond.wait(locker.mutex()); - QVERIFY(thread.wait(five_minutes)); - QVERIFY(thread.isFinished()); - QVERIFY(!thread.isRunning()); - QCOMPARE(thread.result, 0); - delete thread.object; + // very similar to exit() above + { + Quit_Thread thread; + thread.object = new Quit_Object; + thread.object->moveToThread(&thread); + thread.result = -1; + QVERIFY(!thread.isFinished()); + QVERIFY(!thread.isRunning()); - Quit_Thread thread2; - thread2.object = 0; - thread2.result = -1; - QMutexLocker locker2(&thread2.mutex); - thread2.start(); - thread2.quit(); - thread2.cond.wait(locker2.mutex()); - QVERIFY(thread2.wait(five_minutes)); - QCOMPARE(thread2.result, 0); + // start the thread, but keep the event loop from starting + // (while the mutex is locked) + QMutexLocker locker(&thread.mutex); + thread.start(); + QVERIFY(thread.isRunning()); + QVERIFY(!thread.isFinished()); + + // unlock the mutex and let the event loop run + // the Quit_Object above will cause the thread to quit + thread.cond.wait(locker.mutex()); + QVERIFY(thread.wait(five_minutes)); + QVERIFY(thread.isFinished()); + QVERIFY(!thread.isRunning()); + QCOMPARE(thread.result, 0); + delete thread.object; + } + + { + Quit_Thread thread2; + thread2.object = 0; + thread2.result = -1; + + // start the thread, but keep the event loop from starting + // (while the mutex is locked) + QMutexLocker locker2(&thread2.mutex); + thread2.start(); + thread2.quit(); // does nothing, the event loop is not running! + + // unlock the mutex and let the event loop run + thread2.cond.wait(locker2.mutex()); + + // there's no Quit_Object so it won't quit on its own + thread2.quit(); + QVERIFY(thread2.wait(five_minutes)); + QCOMPARE(thread2.result, 0); + } +} + +void tst_QThread::execAfterQuit() +{ + ExecAfterQuitThread thread; + thread.start(); + QVERIFY(thread.wait()); + QCOMPARE(thread.returnValue, 0); } void tst_QThread::wait() @@ -994,8 +1068,17 @@ void tst_QThread::QTBUG15378_exitAndExec() Thread thread; thread.value = 0; thread.start(); - thread.exit(556); - thread.sem1.release(); //should exit the first loop + thread.exit(42); // will do nothing, this value should not appear + thread.sem1.release(); //should enter the first loop + + Exit_Object *exit_object = new Exit_Object; + exit_object->code = 556; + exit_object->thread = &thread; + QMetaObject::invokeMethod(exit_object, "slot", Qt::QueuedConnection); + exit_object->deleteLater(); + exit_object->moveToThread(&thread); // should exit the first loop + exit_object = 0; + thread.sem2.acquire(); int v = thread.value; QCOMPARE(v, 556); -- cgit v0.12 From ead20f4c1edc2e1c5c39f47bf7c9e56600d6362b Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Thu, 27 Jan 2011 16:29:52 +0100 Subject: Fix alignment issue causing crash in QtScript/JavaScriptCore When creating a substring, JSC::UStringImpl required that the base string pointer was 8-byte aligned. However, on platforms where FastMalloc isn't enabled (such as Symbian), it's possible that the system malloc() returns a pointer that is only 4-byte aligned. (On Symbian, this can happen if the argument to malloc() itself isn't a multiple of 8.) Cherry-picked http://trac.webkit.org/changeset/54743 from WebKit trunk, which fixes this issue. (The commit happened shortly after we rebased QtScript/JSC for 4.7, so it applies cleanly to our copy.) Task-number: QTBUG-16828 Reviewed-by: Simon Hausmann --- .../javascriptcore/JavaScriptCore/ChangeLog | 25 +++++ .../JavaScriptCore/runtime/UStringImpl.cpp | 14 +-- .../JavaScriptCore/runtime/UStringImpl.h | 117 ++++++++------------- src/3rdparty/javascriptcore/VERSION | 4 +- 4 files changed, 76 insertions(+), 84 deletions(-) diff --git a/src/3rdparty/javascriptcore/JavaScriptCore/ChangeLog b/src/3rdparty/javascriptcore/JavaScriptCore/ChangeLog index c2b1155..9cbf0c1 100644 --- a/src/3rdparty/javascriptcore/JavaScriptCore/ChangeLog +++ b/src/3rdparty/javascriptcore/JavaScriptCore/ChangeLog @@ -358,6 +358,31 @@ * wtf/AlwaysInline.h: +2010-02-12 Gavin Barraclough + + Reviewed by Darin Adler. + + https://bugs.webkit.org/show_bug.cgi?id=33731 + Many false leaks in release builds due to PtrAndFlags + + Remove UntypedPtrAndBitfield (similar to PtrAndFlags) in UStringImpl, + and steal bits from the refCount instead. + + * runtime/UStringImpl.cpp: + (JSC::UStringImpl::baseSharedBuffer): + (JSC::UStringImpl::~UStringImpl): + * runtime/UStringImpl.h: + (JSC::UStringImpl::cost): + (JSC::UStringImpl::isIdentifier): + (JSC::UStringImpl::setIsIdentifier): + (JSC::UStringImpl::ref): + (JSC::UStringImpl::deref): + (JSC::UStringImpl::UStringImpl): + (JSC::UStringImpl::bufferOwnerString): + (JSC::UStringImpl::bufferOwnership): + (JSC::UStringImpl::isStatic): + (JSC::UStringImpl::): + 2010-02-12 Kwang Yul Seo Reviewed by Adam Barth. diff --git a/src/3rdparty/javascriptcore/JavaScriptCore/runtime/UStringImpl.cpp b/src/3rdparty/javascriptcore/JavaScriptCore/runtime/UStringImpl.cpp index 4b0d1c9..4fde49e 100644 --- a/src/3rdparty/javascriptcore/JavaScriptCore/runtime/UStringImpl.cpp +++ b/src/3rdparty/javascriptcore/JavaScriptCore/runtime/UStringImpl.cpp @@ -38,12 +38,14 @@ namespace JSC { SharedUChar* UStringImpl::baseSharedBuffer() { ASSERT((bufferOwnership() == BufferShared) - || ((bufferOwnership() == BufferOwned) && !m_dataBuffer.asPtr())); + || ((bufferOwnership() == BufferOwned) && !m_buffer)); - if (bufferOwnership() != BufferShared) - m_dataBuffer = UntypedPtrAndBitfield(SharedUChar::create(new OwnFastMallocPtr(m_data)).releaseRef(), BufferShared); + if (bufferOwnership() != BufferShared) { + m_refCountAndFlags = (m_refCountAndFlags & ~s_refCountMaskBufferOwnership) | BufferShared; + m_bufferShared = SharedUChar::create(new OwnFastMallocPtr(m_data)).releaseRef(); + } - return m_dataBuffer.asPtr(); + return m_bufferShared; } SharedUChar* UStringImpl::sharedBuffer() @@ -71,10 +73,10 @@ UStringImpl::~UStringImpl() if (bufferOwnership() == BufferOwned) fastFree(m_data); else if (bufferOwnership() == BufferSubstring) - m_dataBuffer.asPtr()->deref(); + m_bufferSubstring->deref(); else { ASSERT(bufferOwnership() == BufferShared); - m_dataBuffer.asPtr()->deref(); + m_bufferShared->deref(); } } } diff --git a/src/3rdparty/javascriptcore/JavaScriptCore/runtime/UStringImpl.h b/src/3rdparty/javascriptcore/JavaScriptCore/runtime/UStringImpl.h index 4e1ddc7..e6d1a8a 100644 --- a/src/3rdparty/javascriptcore/JavaScriptCore/runtime/UStringImpl.h +++ b/src/3rdparty/javascriptcore/JavaScriptCore/runtime/UStringImpl.h @@ -40,48 +40,6 @@ class IdentifierTable; typedef CrossThreadRefCounted > SharedUChar; -class UntypedPtrAndBitfield { -public: - UntypedPtrAndBitfield() {} - - UntypedPtrAndBitfield(void* ptrValue, uintptr_t bitValue) - : m_value(reinterpret_cast(ptrValue) | bitValue) -#ifndef NDEBUG - , m_leaksPtr(ptrValue) -#endif - { - ASSERT(ptrValue == asPtr()); - ASSERT((*this & ~s_alignmentMask) == bitValue); - } - - template - T asPtr() const { return reinterpret_cast(m_value & s_alignmentMask); } - - UntypedPtrAndBitfield& operator&=(uintptr_t bits) - { - m_value &= bits | s_alignmentMask; - return *this; - } - - UntypedPtrAndBitfield& operator|=(uintptr_t bits) - { - m_value |= bits & ~s_alignmentMask; - return *this; - } - - uintptr_t operator&(uintptr_t mask) const - { - return m_value & mask & ~s_alignmentMask; - } - -private: - static const uintptr_t s_alignmentMask = ~static_cast(0x7); - uintptr_t m_value; -#ifndef NDEBUG - void* m_leaksPtr; // Only used to allow tools like leaks on OSX to detect that the memory is referenced. -#endif -}; - class UStringImpl : Noncopyable { public: template @@ -151,21 +109,27 @@ public: { // For substrings, return the cost of the base string. if (bufferOwnership() == BufferSubstring) - return m_dataBuffer.asPtr()->cost(); + return m_bufferSubstring->cost(); - if (m_dataBuffer & s_reportedCostBit) + if (m_refCountAndFlags & s_refCountFlagHasReportedCost) return 0; - m_dataBuffer |= s_reportedCostBit; + m_refCountAndFlags |= s_refCountFlagHasReportedCost; return m_length; } unsigned hash() const { if (!m_hash) m_hash = computeHash(data(), m_length); return m_hash; } unsigned existingHash() const { ASSERT(m_hash); return m_hash; } // fast path for Identifiers void setHash(unsigned hash) { ASSERT(hash == computeHash(data(), m_length)); m_hash = hash; } // fast path for Identifiers - bool isIdentifier() const { return m_isIdentifier; } - void setIsIdentifier(bool isIdentifier) { m_isIdentifier = isIdentifier; } + bool isIdentifier() const { return m_refCountAndFlags & s_refCountFlagIsIdentifier; } + void setIsIdentifier(bool isIdentifier) + { + if (isIdentifier) + m_refCountAndFlags |= s_refCountFlagIsIdentifier; + else + m_refCountAndFlags &= ~s_refCountFlagIsIdentifier; + } - UStringImpl* ref() { m_refCount += s_refCountIncrement; return this; } - ALWAYS_INLINE void deref() { if (!(m_refCount -= s_refCountIncrement)) delete this; } + UStringImpl* ref() { m_refCountAndFlags += s_refCountIncrement; return this; } + ALWAYS_INLINE void deref() { m_refCountAndFlags -= s_refCountIncrement; if (!(m_refCountAndFlags & s_refCountMask)) delete this; } static void copyChars(UChar* destination, const UChar* source, unsigned numCharacters) { @@ -205,11 +169,10 @@ private: // Used to construct normal strings with an internal or external buffer. UStringImpl(UChar* data, int length, BufferOwnership ownership) : m_data(data) + , m_buffer(0) , m_length(length) - , m_refCount(s_refCountIncrement) + , m_refCountAndFlags(s_refCountIncrement | ownership) , m_hash(0) - , m_isIdentifier(false) - , m_dataBuffer(0, ownership) { ASSERT((ownership == BufferInternal) || (ownership == BufferOwned)); checkConsistency(); @@ -221,11 +184,10 @@ private: enum StaticStringConstructType { ConstructStaticString }; UStringImpl(UChar* data, int length, StaticStringConstructType) : m_data(data) + , m_buffer(0) , m_length(length) - , m_refCount(s_staticRefCountInitialValue) + , m_refCountAndFlags(s_refCountFlagStatic | BufferOwned) , m_hash(0) - , m_isIdentifier(false) - , m_dataBuffer(0, BufferOwned) { checkConsistency(); } @@ -233,28 +195,26 @@ private: // Used to create new strings that are a substring of an existing string. UStringImpl(UChar* data, int length, PassRefPtr base) : m_data(data) + , m_bufferSubstring(base.releaseRef()) , m_length(length) - , m_refCount(s_refCountIncrement) + , m_refCountAndFlags(s_refCountIncrement | BufferSubstring) , m_hash(0) - , m_isIdentifier(false) - , m_dataBuffer(base.releaseRef(), BufferSubstring) { // Do use static strings as a base for substrings; UntypedPtrAndBitfield assumes // that all pointers will be at least 8-byte aligned, we cannot guarantee that of // UStringImpls that are not heap allocated. - ASSERT(m_dataBuffer.asPtr()->size()); - ASSERT(!m_dataBuffer.asPtr()->isStatic()); + ASSERT(m_bufferSubstring->size()); + ASSERT(!m_bufferSubstring->isStatic()); checkConsistency(); } // Used to construct new strings sharing an existing shared buffer. UStringImpl(UChar* data, int length, PassRefPtr sharedBuffer) : m_data(data) + , m_bufferShared(sharedBuffer.releaseRef()) , m_length(length) - , m_refCount(s_refCountIncrement) + , m_refCountAndFlags(s_refCountIncrement | BufferShared) , m_hash(0) - , m_isIdentifier(false) - , m_dataBuffer(sharedBuffer.releaseRef(), BufferShared) { checkConsistency(); } @@ -277,26 +237,31 @@ private: // This number must be at least 2 to avoid sharing empty, null as well as 1 character strings from SmallStrings. static const int s_minLengthToShare = 10; static const unsigned s_copyCharsInlineCutOff = 20; - static const uintptr_t s_bufferOwnershipMask = 3; - static const uintptr_t s_reportedCostBit = 4; // We initialize and increment/decrement the refCount for all normal (non-static) strings by the value 2. // We initialize static strings with an odd number (specifically, 1), such that the refCount cannot reach zero. - static const int s_refCountIncrement = 2; - static const int s_staticRefCountInitialValue = 1; - - UStringImpl* bufferOwnerString() { return (bufferOwnership() == BufferSubstring) ? m_dataBuffer.asPtr() : this; } - const UStringImpl* bufferOwnerString() const { return (bufferOwnership() == BufferSubstring) ? m_dataBuffer.asPtr() : this; } + static const unsigned s_refCountMask = 0xFFFFFFF0; + static const int s_refCountIncrement = 0x20; + static const int s_refCountFlagStatic = 0x10; + static const unsigned s_refCountFlagHasReportedCost = 0x8; + static const unsigned s_refCountFlagIsIdentifier = 0x4; + static const unsigned s_refCountMaskBufferOwnership = 0x3; + + UStringImpl* bufferOwnerString() { return (bufferOwnership() == BufferSubstring) ? m_bufferSubstring : this; } + const UStringImpl* bufferOwnerString() const { return (bufferOwnership() == BufferSubstring) ? m_bufferSubstring : this; } SharedUChar* baseSharedBuffer(); - unsigned bufferOwnership() const { return m_dataBuffer & s_bufferOwnershipMask; } - bool isStatic() const { return m_refCount & 1; } + unsigned bufferOwnership() const { return m_refCountAndFlags & s_refCountMaskBufferOwnership; } + bool isStatic() const { return m_refCountAndFlags & s_refCountFlagStatic; } // unshared data UChar* m_data; + union { + void* m_buffer; + UStringImpl* m_bufferSubstring; + SharedUChar* m_bufferShared; + }; int m_length; - unsigned m_refCount; - mutable unsigned m_hash : 31; - mutable unsigned m_isIdentifier : 1; - UntypedPtrAndBitfield m_dataBuffer; + unsigned m_refCountAndFlags; + mutable unsigned m_hash; JS_EXPORTDATA static UStringImpl* s_null; JS_EXPORTDATA static UStringImpl* s_empty; diff --git a/src/3rdparty/javascriptcore/VERSION b/src/3rdparty/javascriptcore/VERSION index b4744b7..13943b2 100644 --- a/src/3rdparty/javascriptcore/VERSION +++ b/src/3rdparty/javascriptcore/VERSION @@ -4,8 +4,8 @@ This is a snapshot of JavaScriptCore from The commit imported was from the - javascriptcore-snapshot-24012011 branch/tag + javascriptcore-snapshot-27012011 branch/tag and has the sha1 checksum - d143bde5ae8cff229aebd43487a2fce5e713e990 + 3ab0f621048fbeb480b687a28ed31d92d8506150 -- cgit v0.12 From b127b1036ec75c625920a6c029b64a95e3702bf9 Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Fri, 28 Jan 2011 10:10:31 +0100 Subject: Invalidate QScriptPrograms when engine is destroyed If the engine is destroyed before the program, the program must be invalidated; otherwise the program destructor will access a stale engine pointer, which can cause a crash (it crashes on Symbian, but "only" gives a Valgrind warning on Linux for our autotests). We need to keep track of all associated programs, just like we already do for values and strings. This fix follows the exact same pattern, but uses a QSet to keep the patch minimal. No new tests, but the evaluateProgram() test runs successfully on Symbian now, and there are no more Valgrind warnings. Task-number: QTBUG-16987 Reviewed-by: Olivier Goffart --- src/script/api/qscriptengine.cpp | 9 +++++++++ src/script/api/qscriptengine_p.h | 18 ++++++++++++++++++ src/script/api/qscriptprogram.cpp | 21 ++++++++++++++------- src/script/api/qscriptprogram_p.h | 1 + 4 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/script/api/qscriptengine.cpp b/src/script/api/qscriptengine.cpp index 9e338d4..54039c0 100644 --- a/src/script/api/qscriptengine.cpp +++ b/src/script/api/qscriptengine.cpp @@ -1022,6 +1022,7 @@ QScriptEnginePrivate::~QScriptEnginePrivate() while (!ownedAgents.isEmpty()) delete ownedAgents.takeFirst(); + detachAllRegisteredScriptPrograms(); detachAllRegisteredScriptValues(); detachAllRegisteredScriptStrings(); qDeleteAll(m_qobjectData); @@ -1576,6 +1577,14 @@ bool QScriptEnginePrivate::scriptDisconnect(JSC::JSValue signal, JSC::JSValue re #endif +void QScriptEnginePrivate::detachAllRegisteredScriptPrograms() +{ + QSet::const_iterator it; + for (it = registeredScriptPrograms.constBegin(); it != registeredScriptPrograms.constEnd(); ++it) + (*it)->detachFromEngine(); + registeredScriptPrograms.clear(); +} + void QScriptEnginePrivate::detachAllRegisteredScriptValues() { QScriptValuePrivate *it; diff --git a/src/script/api/qscriptengine_p.h b/src/script/api/qscriptengine_p.h index 05a8901..f8144e9 100644 --- a/src/script/api/qscriptengine_p.h +++ b/src/script/api/qscriptengine_p.h @@ -87,6 +87,7 @@ class QScriptEngineAgent; class QScriptEnginePrivate; class QScriptSyntaxCheckResult; class QScriptEngine; +class QScriptProgramPrivate; namespace QScript { @@ -273,6 +274,10 @@ public: static QScriptSyntaxCheckResult checkSyntax(const QString &program); static bool canEvaluate(const QString &program); + inline void registerScriptProgram(QScriptProgramPrivate *program); + inline void unregisterScriptProgram(QScriptProgramPrivate *program); + void detachAllRegisteredScriptPrograms(); + inline QScriptValuePrivate *allocateScriptValuePrivate(size_t); inline void freeScriptValuePrivate(QScriptValuePrivate *p); @@ -368,6 +373,7 @@ public: static const int maxFreeScriptValues = 256; int freeScriptValuesCount; QScriptStringPrivate *registeredScriptStrings; + QSet registeredScriptPrograms; QHash m_typeInfos; int processEventsInterval; QScriptValue abortResult; @@ -566,6 +572,18 @@ inline QByteArray convertToLatin1(const JSC::UString &str) } // namespace QScript +inline void QScriptEnginePrivate::registerScriptProgram(QScriptProgramPrivate *program) +{ + Q_ASSERT(!registeredScriptPrograms.contains(program)); + registeredScriptPrograms.insert(program); +} + +inline void QScriptEnginePrivate::unregisterScriptProgram(QScriptProgramPrivate *program) +{ + Q_ASSERT(registeredScriptPrograms.contains(program)); + registeredScriptPrograms.remove(program); +} + inline QScriptValuePrivate *QScriptEnginePrivate::allocateScriptValuePrivate(size_t size) { if (freeScriptValues) { diff --git a/src/script/api/qscriptprogram.cpp b/src/script/api/qscriptprogram.cpp index da103bb..31af9a0 100644 --- a/src/script/api/qscriptprogram.cpp +++ b/src/script/api/qscriptprogram.cpp @@ -64,6 +64,7 @@ QScriptProgramPrivate::~QScriptProgramPrivate() if (engine) { QScript::APIShim shim(engine); _executable.clear(); + engine->unregisterScriptProgram(this); } } @@ -78,7 +79,10 @@ JSC::EvalExecutable *QScriptProgramPrivate::executable(JSC::ExecState *exec, if (_executable) { if (eng == engine) return _executable.get(); - _executable = 0; + // "Migrating" to another engine; clean up old state + QScript::APIShim shim(engine); + _executable.clear(); + engine->unregisterScriptProgram(this); } WTF::PassRefPtr provider = QScript::UStringSourceProviderWithFeedback::create(sourceCode, fileName, firstLineNumber, eng); @@ -86,10 +90,19 @@ JSC::EvalExecutable *QScriptProgramPrivate::executable(JSC::ExecState *exec, JSC::SourceCode source(provider, firstLineNumber); //after construction of SourceCode provider variable will be null. _executable = JSC::EvalExecutable::create(exec, source); engine = eng; + engine->registerScriptProgram(this); isCompiled = false; return _executable.get(); } +void QScriptProgramPrivate::detachFromEngine() +{ + _executable.clear(); + sourceId = -1; + isCompiled = false; + engine = 0; +} + /*! Constructs a null QScriptProgram. */ @@ -122,9 +135,6 @@ QScriptProgram::QScriptProgram(const QScriptProgram &other) */ QScriptProgram::~QScriptProgram() { - // Q_D(QScriptProgram); - // if (d->engine && (d->ref == 1)) - // d->engine->unregisterScriptProgram(d); } /*! @@ -132,9 +142,6 @@ QScriptProgram::~QScriptProgram() */ QScriptProgram &QScriptProgram::operator=(const QScriptProgram &other) { - // if (d_func() && d_func()->engine && (d_func()->ref == 1)) - // d_func()->engine->unregisterScriptProgram(d_func()); - // } d_ptr = other.d_ptr; return *this; } diff --git a/src/script/api/qscriptprogram_p.h b/src/script/api/qscriptprogram_p.h index d2fd234..e7809ab 100644 --- a/src/script/api/qscriptprogram_p.h +++ b/src/script/api/qscriptprogram_p.h @@ -61,6 +61,7 @@ public: JSC::EvalExecutable *executable(JSC::ExecState *exec, QScriptEnginePrivate *engine); + void detachFromEngine(); QBasicAtomicInt ref; -- cgit v0.12 From 298b54b4fa99db47d62ec37eacefb4f419b79a69 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Fri, 28 Jan 2011 11:12:50 +0100 Subject: Revert "Improve timer ID safety by using a serial counter per ID." This reverts commit 121e2b39043a4ffc6583f250aebb9a3a746076c1. It was considered too dangerous for 4.7. --- src/corelib/kernel/qabstracteventdispatcher.cpp | 30 +++++-------------------- 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/src/corelib/kernel/qabstracteventdispatcher.cpp b/src/corelib/kernel/qabstracteventdispatcher.cpp index cc08f092..e79f87a 100644 --- a/src/corelib/kernel/qabstracteventdispatcher.cpp +++ b/src/corelib/kernel/qabstracteventdispatcher.cpp @@ -137,12 +137,6 @@ void QAbstractEventDispatcherPrivate::init() // free list). As an added protection, we use the cell to store an invalid // (negative) value that we can later check for integrity. // -// ABA prevention simply adds a value to 7 of the top 8 bits when resetting -// nextFreeTimerId. -// -// The extra code is the bucket allocation which allows us to start with a -// very small bucket size and grow as needed. -// // (continues below). int QAbstractEventDispatcherPrivate::allocateTimerId() { @@ -170,8 +164,6 @@ int QAbstractEventDispatcherPrivate::allocateTimerId() newTimerId = prepareNewValueWithSerialNumber(timerId, b[at]); } while (!nextFreeTimerId.testAndSetRelaxed(timerId, newTimerId)); - timerId &= TimerIdMask; - timerId |= b[at] & TimerSerialMask; b[at] = -timerId; return timerId; @@ -182,13 +174,12 @@ int QAbstractEventDispatcherPrivate::allocateTimerId() // X[timerId] = nextFreeTimerId; // then we update nextFreeTimerId to the timer we've just released // +// The extra code in allocateTimerId and releaseTimerId are ABA prevention +// and bucket memory. The buckets are simply to make sure we allocate only +// the necessary number of timers. See above. +// // ABA prevention simply adds a value to 7 of the top 8 bits when resetting // nextFreeTimerId. -// -// In addition to that, we update the same 7 bits in each entry in the bucket -// as a counter. That way, a timer ID allocated and released will always be -// returned with a different ID. This reduces the chances of timers being released -// erroneously by application code. void QAbstractEventDispatcherPrivate::releaseTimerId(int timerId) { int which = timerId & TimerIdMask; @@ -196,21 +187,12 @@ void QAbstractEventDispatcherPrivate::releaseTimerId(int timerId) int at = bucketIndex(bucket, which); int *b = timerIds[bucket]; -#ifndef QT_NO_DEBUG - // debug code - Q_ASSERT_X(timerId == -b[at], "QAbstractEventDispatcher::releaseTimerId", "Timer ID was not found, fix application"); -#else - if (timerId != -b[at]) { - // release code - qWarning("Timer ID %d was not found, fix application", timerId); - return; - } -#endif + Q_ASSERT(b[at] == -timerId); int freeId, newTimerId; do { freeId = nextFreeTimerId;//.loadAcquire(); // ### FIXME Proper memory ordering semantics - b[at] = prepareNewValueWithSerialNumber(-b[at], freeId); + b[at] = freeId & TimerIdMask; newTimerId = prepareNewValueWithSerialNumber(freeId, timerId); } while (!nextFreeTimerId.testAndSetRelease(freeId, newTimerId)); -- cgit v0.12 From bdf3782b40b0fc2ebfda960be08c90b549cfd970 Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Fri, 28 Jan 2011 12:23:08 +0100 Subject: Fix potential networking crash due to null-pointer dereference An internal bug report suggests that we unconditionally dereference the backend pointer in QNetworkReplyImpl when checking for the synchronity of the originating request. The dereferencing code was introduced in commit ad1e82323225e996720136e8b2d669166b8d8441. Unfortunately the report does not detail where/how the crash happened, but it appears plausible that the backend pointer became null, and the surrounding code that has extra checks suggests this, too. In an attempt of defensive programming this patch introduces the missing check in the reported line 112 as well as in other places where it seems appropriate. Reviewed-by: Peter Hartmann --- src/network/access/qnetworkreplyimpl.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/network/access/qnetworkreplyimpl.cpp b/src/network/access/qnetworkreplyimpl.cpp index 9d7082c..343f344 100644 --- a/src/network/access/qnetworkreplyimpl.cpp +++ b/src/network/access/qnetworkreplyimpl.cpp @@ -109,7 +109,7 @@ void QNetworkReplyImplPrivate::_q_startOperation() } #endif - if (backend->isSynchronous()) { + if (backend && backend->isSynchronous()) { state = Finished; } else { if (state != Finished) { @@ -296,7 +296,7 @@ void QNetworkReplyImplPrivate::setup(QNetworkAccessManager::Operation op, const // in QtWebKit. QVariant synchronousHttpAttribute = req.attribute( static_cast(QNetworkRequest::DownloadBufferAttribute + 1)); - if (synchronousHttpAttribute.toBool()) { + if (backend && synchronousHttpAttribute.toBool()) { backend->setSynchronous(true); if (outgoingData && outgoingData->isSequential()) { outgoingDataBuffer = new QRingBuffer(); @@ -351,7 +351,7 @@ void QNetworkReplyImplPrivate::setup(QNetworkAccessManager::Operation op, const QMetaObject::invokeMethod(q, "_q_startOperation", Qt::QueuedConnection); } #else - if (backend->isSynchronous()) + if (backend && backend->isSynchronous()) _q_startOperation(); else QMetaObject::invokeMethod(q, "_q_startOperation", Qt::QueuedConnection); -- cgit v0.12