From: Heeyong Song Date: Tue, 29 Aug 2017 02:33:35 +0000 (+0900) Subject: Revert "Change Dali::Thread, Dali::Mutex and Dali::ConditionalWait to use std::thread... X-Git-Tag: dali_1.2.55~2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=fe46fd3de42420ef17451c2572792751e7b4c6d2;p=platform%2Fcore%2Fuifw%2Fdali-core.git Revert "Change Dali::Thread, Dali::Mutex and Dali::ConditionalWait to use std::thread from C++ 11 instead of pthread APIs" This reverts commit c659ac010b256880530f33e657876ffbee95b769. It caused GBS build error in Tizen 4.0. Change-Id: I50858dda10d27b0d928d087c26c6e1b125925170 --- diff --git a/build/tizen/dali-core/Makefile.am b/build/tizen/dali-core/Makefile.am index 137b6f6..ad081d5 100644 --- a/build/tizen/dali-core/Makefile.am +++ b/build/tizen/dali-core/Makefile.am @@ -53,7 +53,8 @@ libdali_core_la_CXXFLAGS = -DDALI_COMPILATION \ $(dali_core_includes) \ $(DALI_CFLAGS) -libdali_core_la_LIBADD = $(DALI_LDFLAGS) +libdali_core_la_LIBADD = $(DALI_LDFLAGS) \ + -lpthread # Install headers under the correct subdirectories platformabstractiondir = $(includedir)/dali/integration-api diff --git a/dali/devel-api/threading/conditional-wait.cpp b/dali/devel-api/threading/conditional-wait.cpp index fa0a86c..adc5760 100644 --- a/dali/devel-api/threading/conditional-wait.cpp +++ b/dali/devel-api/threading/conditional-wait.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017 Samsung Electronics Co., Ltd. + * Copyright (c) 2015 Samsung Electronics Co., Ltd. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,78 +19,77 @@ #include // EXTERNAL INCLUDES -#include -#include +#include // INTERNAL INCLUDES #include -#include +#include namespace Dali { -/** - * Data members for ConditionalWait - */ + struct ConditionalWait::ConditionalWaitImpl { - std::mutex mutex; - std::condition_variable condition; + pthread_mutex_t mutex; + pthread_cond_t condition; volatile unsigned int count; }; -/** - * Data members for ConditionalWait::ScopedLock - */ -struct ConditionalWait::ScopedLock::ScopedLockImpl -{ - ScopedLockImpl( ConditionalWait& wait ) - : wait( wait ), - lock( wait.mImpl->mutex ) // locks for the lifecycle of this object - { } - ConditionalWait& wait; - std::unique_lock lock; -}; -ConditionalWait::ScopedLock::ScopedLock( ConditionalWait& wait ) -: mImpl( new ConditionalWait::ScopedLock::ScopedLockImpl( wait ) ) +ConditionalWait::ScopedLock::ScopedLock( ConditionalWait& wait ) : mWait(wait) { - Internal::MutexTrace::Lock(); // matching sequence in mutex.cpp + Internal::Mutex::Lock( &wait.mImpl->mutex ); } ConditionalWait::ScopedLock::~ScopedLock() { - Internal::MutexTrace::Unlock(); - delete mImpl; -} - -ConditionalWait& ConditionalWait::ScopedLock::GetLockedWait() const -{ - return mImpl->wait; // mImpl can never be NULL + ConditionalWait& wait = mWait; + Internal::Mutex::Unlock( &wait.mImpl->mutex ); } ConditionalWait::ConditionalWait() : mImpl( new ConditionalWaitImpl ) { + if( pthread_mutex_init( &mImpl->mutex, NULL ) ) + { + DALI_LOG_ERROR( "Unable to initialise mutex in ConditionalWait\n" ); + } + if( pthread_cond_init( &mImpl->condition, NULL ) ) + { + DALI_LOG_ERROR( "Unable to initialise conditional\n" ); + } mImpl->count = 0; } ConditionalWait::~ConditionalWait() { + if( pthread_cond_destroy( &mImpl->condition ) ) + { + DALI_LOG_ERROR( "Unable to destroy conditional\n" ); + } + if( pthread_mutex_destroy( &mImpl->mutex ) ) + { + DALI_LOG_ERROR( "Unable to destroy mutex in ConditionalWait\n" ); + } delete mImpl; } void ConditionalWait::Notify() { - // conditional wait requires a lock to be held - ScopedLock lock( *this ); + // pthread_cond_wait requires a lock to be held + Internal::Mutex::Lock( &mImpl->mutex ); volatile unsigned int previousCount = mImpl->count; mImpl->count = 0; // change state before broadcast as that may wake clients immediately - // notify does nothing if the thread is not waiting but still has a system call overhead - // notify all threads to continue + // broadcast does nothing if the thread is not waiting but still has a system call overhead + // broadcast all threads to continue if( 0 != previousCount ) { - mImpl->condition.notify_all(); // no return value + if( pthread_cond_broadcast( &mImpl->condition ) ) + { + DALI_LOG_ERROR( "Error calling pthread_cond_broadcast\n" ); + } } + Internal::Mutex::Unlock( &mImpl->mutex ); } void ConditionalWait::Notify( const ScopedLock& scope ) @@ -100,26 +99,35 @@ void ConditionalWait::Notify( const ScopedLock& scope ) volatile unsigned int previousCount = mImpl->count; mImpl->count = 0; // change state before broadcast as that may wake clients immediately - // notify does nothing if the thread is not waiting but still has a system call overhead - // notify all threads to continue + // broadcast does nothing if the thread is not waiting but still has a system call overhead + // broadcast all threads to continue if( 0 != previousCount ) { - mImpl->condition.notify_all(); // no return value + if( pthread_cond_broadcast( &mImpl->condition ) ) + { + DALI_LOG_ERROR( "Error calling pthread_cond_broadcast\n" ); + } } } void ConditionalWait::Wait() { - // conditional wait requires a lock to be held - ScopedLock scope( *this ); + // pthread_cond_wait requires a lock to be held + Internal::Mutex::Lock( &mImpl->mutex ); ++(mImpl->count); - // conditional wait may wake up without anyone calling Notify + // pthread_cond_wait may wake up without anyone calling Notify do { // wait while condition changes - mImpl->condition.wait( scope.mImpl->lock ); // need to pass in the std::unique_lock + if( pthread_cond_wait( &mImpl->condition, &mImpl->mutex ) ) // releases the lock whilst waiting + { + DALI_LOG_ERROR( "Error calling pthread_cond_wait\n" ); + break; + } } while( 0 != mImpl->count ); + // when condition returns the mutex is locked so release the lock + Internal::Mutex::Unlock( &mImpl->mutex ); } void ConditionalWait::Wait( const ScopedLock& scope ) @@ -129,11 +137,16 @@ void ConditionalWait::Wait( const ScopedLock& scope ) ++(mImpl->count); - // conditional wait may wake up without anyone calling Notify + // pthread_cond_wait may wake up without anyone calling Notify so loop until + // count has been reset in a notify: do { // wait while condition changes - mImpl->condition.wait( scope.mImpl->lock ); // need to pass in the std::unique_lock + if( pthread_cond_wait( &mImpl->condition, &mImpl->mutex ) ) // releases the lock whilst waiting + { + DALI_LOG_ERROR( "Error calling pthread_cond_wait\n" ); + break; + } } while( 0 != mImpl->count ); diff --git a/dali/devel-api/threading/conditional-wait.h b/dali/devel-api/threading/conditional-wait.h index 81660cc..2cbd289 100644 --- a/dali/devel-api/threading/conditional-wait.h +++ b/dali/devel-api/threading/conditional-wait.h @@ -2,7 +2,7 @@ #define __DALI_CONDITIONAL_WAIT_H__ /* - * Copyright (c) 2017 Samsung Electronics Co., Ltd. + * Copyright (c) 2015 Samsung Electronics Co., Ltd. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -55,18 +55,15 @@ public: * Getter for the ConditionalWait locked for this instance's lifetime. * @return the ConditionalWait object currently locked. */ - ConditionalWait& GetLockedWait() const; - - public: // Data, public to allow nesting class to access - - struct ScopedLockImpl; - ScopedLockImpl* mImpl; + ConditionalWait& GetLockedWait() const { return mWait; } private: // Not implemented as ScopedLock cannot be copied: ScopedLock( const ScopedLock& ); const ScopedLock& operator=( const ScopedLock& ); + + ConditionalWait& mWait; }; /** diff --git a/dali/devel-api/threading/mutex.cpp b/dali/devel-api/threading/mutex.cpp index 0f1770a..4e0cb30 100644 --- a/dali/devel-api/threading/mutex.cpp +++ b/dali/devel-api/threading/mutex.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017 Samsung Electronics Co., Ltd. + * Copyright (c) 2015 Samsung Electronics Co., Ltd. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,29 +19,37 @@ #include // EXTERNAL INCLUDES -#include +#include // INTERNAL INCLUDES #include -#include +#include namespace Dali { struct Mutex::MutexImpl { - std::mutex mutex; + pthread_mutex_t mutex; bool locked; }; Mutex::Mutex() : mImpl( new MutexImpl ) { + if( pthread_mutex_init( &mImpl->mutex, NULL ) ) + { + DALI_LOG_ERROR( "Unable to initialise Mutex\n" ); + } mImpl->locked = false; } Mutex::~Mutex() { + if( pthread_mutex_destroy( &mImpl->mutex ) ) + { + DALI_LOG_ERROR( "Unable to destroy Mutex\n" ); + } // nothing else to do as there is no Lock/Unlock API // ScopedLock destructor will always unlock the mutex delete mImpl; @@ -55,16 +63,14 @@ bool Mutex::IsLocked() Mutex::ScopedLock::ScopedLock( Mutex& mutex ) : mMutex( mutex ) { - mMutex.mImpl->mutex.lock(); - Internal::MutexTrace::Lock(); // matching sequence in conditional-wait.cpp + Internal::Mutex::Lock( &mMutex.mImpl->mutex ); mMutex.mImpl->locked = true; } Mutex::ScopedLock::~ScopedLock() { mMutex.mImpl->locked = false; - Internal::MutexTrace::Unlock(); // reverse sequence from lock - mMutex.mImpl->mutex.unlock(); + Internal::Mutex::Unlock( &mMutex.mImpl->mutex ); } } // namespace Dali diff --git a/dali/devel-api/threading/mutex.h b/dali/devel-api/threading/mutex.h index 9490212..cd41f9a 100644 --- a/dali/devel-api/threading/mutex.h +++ b/dali/devel-api/threading/mutex.h @@ -2,7 +2,7 @@ #define __DALI_MUTEX_H__ /* - * Copyright (c) 2017 Samsung Electronics Co., Ltd. + * Copyright (c) 2015 Samsung Electronics Co., Ltd. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/dali/devel-api/threading/thread.cpp b/dali/devel-api/threading/thread.cpp index be4077d..d7bfa0b 100644 --- a/dali/devel-api/threading/thread.cpp +++ b/dali/devel-api/threading/thread.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017 Samsung Electronics Co., Ltd. + * Copyright (c) 2015 Samsung Electronics Co., Ltd. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,7 +20,7 @@ // EXTERNAL INCLUDES #include -#include +#include #include namespace Dali @@ -28,45 +28,43 @@ namespace Dali struct Thread::ThreadImpl { - ThreadImpl( Thread& aThis ) - : thread( &Thread::InternalThreadEntryFunc, std::ref( aThis ) ) - { - // std::thread starts execution immediately - } - ~ThreadImpl( ) - { - thread.join(); - } - std::thread thread; + pthread_t thread; + bool isCreated; }; Thread::Thread() -: mImpl( nullptr ) +: mImpl( new ThreadImpl ) { + mImpl->isCreated = false; } Thread::~Thread() { - Join(); + delete mImpl; } void Thread::Start() { - if( !mImpl ) - { - mImpl = new Thread::ThreadImpl( *this ); - } + DALI_ASSERT_DEBUG( !mImpl->isCreated ); + + int error = pthread_create( &(mImpl->thread), NULL, InternalThreadEntryFunc, this ); + DALI_ASSERT_ALWAYS( !error && "Failed to create a new thread" ); + mImpl->isCreated = true; } void Thread::Join() { - delete mImpl; - mImpl = nullptr; + if( mImpl->isCreated ) + { + mImpl->isCreated = false; + pthread_join( mImpl->thread, NULL ); + } } -void Thread::InternalThreadEntryFunc( Thread& aThis ) +void* Thread::InternalThreadEntryFunc( void* This ) { - aThis.Run(); + ( static_cast( This ) )->Run(); + return NULL; } } // namespace Dali diff --git a/dali/devel-api/threading/thread.h b/dali/devel-api/threading/thread.h index 0e21639..535e700 100644 --- a/dali/devel-api/threading/thread.h +++ b/dali/devel-api/threading/thread.h @@ -2,7 +2,7 @@ #define __DALI_THREAD_H__ /* - * Copyright (c) 2017 Samsung Electronics Co., Ltd. + * Copyright (c) 2015 Samsung Electronics Co., Ltd. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,7 @@ // INTERNAL INCLUDES #include + /** * The top level DALi namespace */ @@ -65,9 +66,9 @@ private: /** * Helper for the thread calling the entry function. - * @param[in] This A pointer to the current thread object + * @param[in] This A pointer to the current RenderThread object */ - static void InternalThreadEntryFunc( Thread& This ); + static void* InternalThreadEntryFunc( void* This ); // Undefined Thread( const Thread& ); @@ -78,7 +79,6 @@ private: struct ThreadImpl; ThreadImpl* mImpl; - }; } diff --git a/dali/internal/common/mutex-trace.cpp b/dali/internal/common/mutex-impl.cpp similarity index 89% rename from dali/internal/common/mutex-trace.cpp rename to dali/internal/common/mutex-impl.cpp index c96b72c..9c85e83 100644 --- a/dali/internal/common/mutex-trace.cpp +++ b/dali/internal/common/mutex-impl.cpp @@ -16,7 +16,8 @@ */ // HEADER -#include +#include + #ifdef LOCK_BACKTRACE_ENABLED // EXTERNAL INCLUDES @@ -41,7 +42,7 @@ extern std::string Demangle( const char* symbol ); namespace Internal { -namespace MutexTrace +namespace Mutex { namespace @@ -65,7 +66,7 @@ thread_local BackTraceInfo gBackTraceInfo[ MAX_LOCK_SUPPORT ]; ///< Thread local thread_local unsigned int gThreadLockCount = 0; ///< Thread local storage for the backtrace of the locks } // unnamed namespace -void Lock() +void Lock( pthread_mutex_t* mutex ) { ++gThreadLockCount; @@ -105,14 +106,23 @@ void Lock() //DALI_ASSERT_DEBUG( gThreadLockCount == 1 ); #endif // LOCK_BACKTRACE_ENABLED + + if( pthread_mutex_lock( mutex ) ) + { + DALI_LOG_ERROR( "Error calling pthread_mutex_lock\n" ); + } } -void Unlock() +void Unlock( pthread_mutex_t* mutex ) { + if( pthread_mutex_unlock( mutex ) ) + { + DALI_LOG_ERROR( "Error calling pthread_mutex_unlock\n" ); + } --gThreadLockCount; } -} // namespace MutexTrace +} // namespace Mutex } // namespace Internal diff --git a/dali/internal/common/mutex-trace.h b/dali/internal/common/mutex-impl.h similarity index 65% rename from dali/internal/common/mutex-trace.h rename to dali/internal/common/mutex-impl.h index 12c856f..bcf07e9 100644 --- a/dali/internal/common/mutex-trace.h +++ b/dali/internal/common/mutex-impl.h @@ -1,8 +1,8 @@ -#ifndef DALI_INTERNAL_MUTEX_TRACE_H -#define DALI_INTERNAL_MUTEX_TRACE_H +#ifndef __DALI_INTERNAL_MUTEX_H__ +#define __DALI_INTERNAL_MUTEX_H__ /* - * Copyright (c) 2017 Samsung Electronics Co., Ltd. + * Copyright (c) 2015 Samsung Electronics Co., Ltd. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,9 @@ * */ +// EXTERNAL INCLUDES +#include + namespace Dali { @@ -32,25 +35,33 @@ namespace Internal * * @note lock backtrace needs to be enabled to see the warnings. */ -namespace MutexTrace +namespace Mutex { /** - * @brief Increments a thread-local storage counter. + * @brief Locks the given mutex. + * + * Increments a thread-local storage counter. + * + * @param A pointer to the mutex that should be locked. * * @note If the counter is > 1 and lock backtrace is enabled, then the backtrace for all locks will be shown as a warning. */ -void Lock(); +void Lock( pthread_mutex_t* mutex ); /** - * @brief Decrements a thread-local storage counter + * @brief Unlocks the given mutex. + * + * @param A pointer to the mutex that should be unlocked. + * + * Decrements a thread-local storage counter. */ -void Unlock(); +void Unlock( pthread_mutex_t* mutex ); -} // namespace MutexTrace +} // namespace Mutex } // namespace Internal } // namespace Dali -#endif // DALI_INTERNAL_MUTEX_TRACE_H +#endif // __DALI_INTERNAL_MUTEX_H__ diff --git a/dali/internal/file.list b/dali/internal/file.list index bae7b47..4b98598 100644 --- a/dali/internal/file.list +++ b/dali/internal/file.list @@ -6,7 +6,7 @@ internal_src_files = \ $(internal_src_dir)/common/internal-constants.cpp \ $(internal_src_dir)/common/math.cpp \ $(internal_src_dir)/common/message-buffer.cpp \ - $(internal_src_dir)/common/mutex-trace.cpp \ + $(internal_src_dir)/common/mutex-impl.cpp \ $(internal_src_dir)/common/image-sampler.cpp \ $(internal_src_dir)/common/image-attributes.cpp \ $(internal_src_dir)/common/fixed-size-memory-pool.cpp \ diff --git a/packaging/dali.spec b/packaging/dali.spec index 4ae6727..5b608ed 100644 --- a/packaging/dali.spec +++ b/packaging/dali.spec @@ -67,7 +67,7 @@ Integration development package for the OpenGLES Canvas - headers for integratin %build PREFIX="/usr" CXXFLAGS+=" -Wall -g -Os -DNDEBUG -fPIC -fvisibility-inlines-hidden -fdata-sections -ffunction-sections " -LDFLAGS+=" -Wl,--rpath=$PREFIX/lib -Wl,--as-needed -Wl,--gc-sections -lgcc_s -lgcc -Wl,-Bsymbolic-functions " +LDFLAGS+=" -Wl,--rpath=$PREFIX/lib -Wl,--as-needed -Wl,--gc-sections -lgcc_s -lgcc -lpthread -Wl,-Bsymbolic-functions " %ifarch %{arm} CXXFLAGS+=" -D_ARCH_ARM_ -mfpu=neon"