From a0a03da71a2167873c600a452075ae13f29cc113 Mon Sep 17 00:00:00 2001 From: kaetemi Date: Wed, 11 Apr 2012 20:20:02 +0200 Subject: [PATCH 1/5] Added: Some sort of implementation for CPThread::isRunning() --- code/nel/include/nel/misc/p_thread.h | 3 ++- code/nel/src/misc/p_thread.cpp | 37 ++++++++++++++++------------ 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/code/nel/include/nel/misc/p_thread.h b/code/nel/include/nel/misc/p_thread.h index 82f6a4ee3..a4db1f55f 100644 --- a/code/nel/include/nel/misc/p_thread.h +++ b/code/nel/include/nel/misc/p_thread.h @@ -59,8 +59,9 @@ public: /// Internal use IRunnable *Runnable; + uint8 _StateV2; // 0=not created, 1=started, 2=ended, 3=finished + private: - uint8 _State; // 0=not created, 1=started, 2=finished uint32 _StackSize; pthread_t _ThreadHandle; }; diff --git a/code/nel/src/misc/p_thread.cpp b/code/nel/src/misc/p_thread.cpp index e20981c98..77866471e 100644 --- a/code/nel/src/misc/p_thread.cpp +++ b/code/nel/src/misc/p_thread.cpp @@ -84,6 +84,8 @@ static void *ProxyFunc( void *arg ) // Run the code of the thread parent->Runnable->run(); + parent->_StateV2 = 2; + // Allow some clean // pthread_exit(0); return NULL; @@ -96,7 +98,7 @@ static void *ProxyFunc( void *arg ) */ CPThread::CPThread(IRunnable *runnable, uint32 stackSize) : Runnable(runnable), - _State(0), + _StateV2(0), _StackSize(stackSize) {} @@ -106,10 +108,10 @@ CPThread::CPThread(IRunnable *runnable, uint32 stackSize) */ CPThread::~CPThread() { - if(_State == 1) + if(_StateV2 == 1 || _StateV2 == 2) terminate(); // force the end of the thread if not already ended - if(_State > 0) + if(_StateV2 > 0) pthread_detach(_ThreadHandle); // free allocated resources only if it was created } @@ -132,13 +134,14 @@ void CPThread::start() { throw EThread("Cannot start new thread"); } - _State = 1; + _StateV2 = 1; } bool CPThread::isRunning() { - // TODO : need a real implementation here that check thread status - return _State == 1; + // ExTODO : need a real implementation here that check thread status + // DONE : some sort of implementation + return _StateV2 == 1; } /* @@ -146,11 +149,11 @@ bool CPThread::isRunning() */ void CPThread::terminate() { - if(_State == 1) + if (_StateV2 == 1 || _StateV2 == 2) { // cancel only if started pthread_cancel(_ThreadHandle); - _State = 2; // set to finished + _StateV2 = 3; // set to finished } } @@ -159,13 +162,13 @@ void CPThread::terminate() */ void CPThread::wait () { - if(_State == 1) + if (_StateV2 == 1 || _StateV2 == 2) { if(pthread_join(_ThreadHandle, 0) != 0) { throw EThread( "Cannot join with thread" ); } - _State = 2; // set to finished + _StateV2 = 3; // set to finished } } @@ -209,27 +212,29 @@ uint64 CPThread::getCPUMask() void CPThread::setPriority(TThreadPriority priority) { - // TODO: Verify and test this + // TODO: Test this + sched_param sp; switch (priority) { case ThreadPriorityHigh: { int minPrio = sched_get_priority_min(SCHED_FIFO); int maxPrio = sched_get_priority_max(SCHED_FIFO); - int prio = ((maxPrio - minPrio) / 4) + minPrio; - pthread_setschedparam(_ThreadHandle, SCHED_FIFO, prio); + sp.sched_priority = ((maxPrio - minPrio) / 4) + minPrio; + pthread_setschedparam(_ThreadHandle, SCHED_FIFO, &sp); break; } case ThreadPriorityHighest: { int minPrio = sched_get_priority_min(SCHED_FIFO); int maxPrio = sched_get_priority_max(SCHED_FIFO); - int prio = ((maxPrio - minPrio) / 2) + minPrio; - pthread_setschedparam(_ThreadHandle, SCHED_FIFO, prio); + sp.sched_priority = ((maxPrio - minPrio) / 2) + minPrio; + pthread_setschedparam(_ThreadHandle, SCHED_FIFO, &sp); break; } default: - pthread_setschedparam(_ThreadHandle, SCHED_OTHER, 0); + sp.sched_priority = 0; + pthread_setschedparam(_ThreadHandle, SCHED_OTHER, &sp); } } From 20e3ce42a37840769090cd581761798f655731e1 Mon Sep 17 00:00:00 2001 From: kaetemi Date: Wed, 11 Apr 2012 21:55:33 +0200 Subject: [PATCH 2/5] Fixed: Implementation for CPThread::isRunning() --- code/nel/include/nel/misc/p_thread.h | 10 +++- code/nel/src/misc/p_thread.cpp | 85 +++++++++++++++++++++------- 2 files changed, 72 insertions(+), 23 deletions(-) diff --git a/code/nel/include/nel/misc/p_thread.h b/code/nel/include/nel/misc/p_thread.h index a4db1f55f..7e6a4d5a5 100644 --- a/code/nel/include/nel/misc/p_thread.h +++ b/code/nel/include/nel/misc/p_thread.h @@ -36,6 +36,12 @@ namespace NLMISC { class CPThread : public IThread { public: + enum TThreadState + { + ThreadStateNone, + ThreadStateRunning, + ThreadStateFinished, + }; /// Constructor CPThread( IRunnable *runnable, uint32 stackSize); @@ -59,11 +65,11 @@ public: /// Internal use IRunnable *Runnable; - uint8 _StateV2; // 0=not created, 1=started, 2=ended, 3=finished + TThreadState _State; + pthread_t _ThreadHandle; private: uint32 _StackSize; - pthread_t _ThreadHandle; }; /** diff --git a/code/nel/src/misc/p_thread.cpp b/code/nel/src/misc/p_thread.cpp index 77866471e..9524ca0bb 100644 --- a/code/nel/src/misc/p_thread.cpp +++ b/code/nel/src/misc/p_thread.cpp @@ -84,7 +84,16 @@ static void *ProxyFunc( void *arg ) // Run the code of the thread parent->Runnable->run(); - parent->_StateV2 = 2; + { + pthread_t thread_self = pthread_self(); + // Make sure the parent still cares + // If this thread was replaced with a new thread (which should not happen), + // and the IThread object has been deleted, this will likely crash. + if (parent->_ThreadHandle == thread_self) + parent->_State = CPThread::ThreadStateFinished; + else + throw EThread("Thread ended after being detached, this should not happen"); + } // Allow some clean // pthread_exit(0); @@ -98,7 +107,7 @@ static void *ProxyFunc( void *arg ) */ CPThread::CPThread(IRunnable *runnable, uint32 stackSize) : Runnable(runnable), - _StateV2(0), + _State(ThreadStateNone), _StackSize(stackSize) {} @@ -108,10 +117,9 @@ CPThread::CPThread(IRunnable *runnable, uint32 stackSize) */ CPThread::~CPThread() { - if(_StateV2 == 1 || _StateV2 == 2) - terminate(); // force the end of the thread if not already ended + terminate(); // force the end of the thread if not already ended - if(_StateV2 > 0) + if (_State != ThreadStateNone) pthread_detach(_ThreadHandle); // free allocated resources only if it was created } @@ -121,27 +129,51 @@ CPThread::~CPThread() void CPThread::start() { pthread_attr_t tattr; - pthread_t tid; int ret; - /* initialized with default attributes */ - ret = pthread_attr_init(&tattr); + if (_StackSize != 0) + { + /* initialized with default attributes */ + ret = pthread_attr_init(&tattr); - /* setting the size of the stack also */ - ret = pthread_attr_setstacksize(&tattr, _StackSize); + /* setting the size of the stack also */ + ret = pthread_attr_setstacksize(&tattr, _StackSize); + } + + bool detach_old_thread = false; + pthread_t old_thread_handle; + if (_State != ThreadStateNone) + { + if (_State == ThreadStateRunning) + { + // I don't know if this behaviour is allowed, but neither thread implementations + // check the start function, and both simply let the existing running thread for what it is... + // From now on, this is not allowed. + throw EThread("Starting a thread that is already started, existing thread will continue running, this should not happen"); + } + detach_old_thread = true; + old_thread_handle = _ThreadHandle; + } - if(pthread_create(&_ThreadHandle, _StackSize != 0 ? &tattr : 0, ProxyFunc, this) != 0) + if (pthread_create(&_ThreadHandle, _StackSize != 0 ? &tattr : NULL, ProxyFunc, this) != 0) { throw EThread("Cannot start new thread"); } - _StateV2 = 1; + _State = ThreadStateRunning; + + if (detach_old_thread) + { + // Docs don't say anything about what happens when pthread_create is called with existing handle referenced. + if (old_thread_handle == _ThreadHandle) + throw EThread("Thread handle did not change, this should not happen"); + // Don't care about old thread, free resources when it terminates. + pthread_detach(old_thread_handle); + } } bool CPThread::isRunning() { - // ExTODO : need a real implementation here that check thread status - // DONE : some sort of implementation - return _StateV2 == 1; + return _State == ThreadStateRunning; } /* @@ -149,11 +181,11 @@ bool CPThread::isRunning() */ void CPThread::terminate() { - if (_StateV2 == 1 || _StateV2 == 2) + if (_State == ThreadStateRunning) { // cancel only if started pthread_cancel(_ThreadHandle); - _StateV2 = 3; // set to finished + _State = ThreadStateFinished; // set to finished } } @@ -162,13 +194,24 @@ void CPThread::terminate() */ void CPThread::wait () { - if (_StateV2 == 1 || _StateV2 == 2) + if (_State == ThreadStateRunning) { - if(pthread_join(_ThreadHandle, 0) != 0) + int error = pthread_join(_ThreadHandle, 0) != 0; + switch (error) { - throw EThread( "Cannot join with thread" ); + case 0: + break; + case EINVAL: + throw EThread("Thread is not joinable"); + case ESRCH: + throw EThread("No thread found with this id"); + case EDEADLK: + throw EThread("Deadlock detected or calling thread waits for itself"); + default: + throw EThread("Unknown thread join error"); } - _StateV2 = 3; // set to finished + if(_State != ThreadStateFinished) + throw EThread("Thread did not finish, this should not happen"); } } From 83ce48e8dd994e3cde2f7924e43f34e74fa1c737 Mon Sep 17 00:00:00 2001 From: kaetemi Date: Wed, 11 Apr 2012 22:05:18 +0200 Subject: [PATCH 3/5] Changed: Make CWinThread::start more sane, and fixed a typo --- code/nel/src/misc/p_thread.cpp | 2 +- code/nel/src/misc/win_thread.cpp | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/code/nel/src/misc/p_thread.cpp b/code/nel/src/misc/p_thread.cpp index 9524ca0bb..f752f4bdf 100644 --- a/code/nel/src/misc/p_thread.cpp +++ b/code/nel/src/misc/p_thread.cpp @@ -196,7 +196,7 @@ void CPThread::wait () { if (_State == ThreadStateRunning) { - int error = pthread_join(_ThreadHandle, 0) != 0; + int error = pthread_join(_ThreadHandle, 0); switch (error) { case 0: diff --git a/code/nel/src/misc/win_thread.cpp b/code/nel/src/misc/win_thread.cpp index f8a961a74..c9bfc90a1 100644 --- a/code/nel/src/misc/win_thread.cpp +++ b/code/nel/src/misc/win_thread.cpp @@ -190,6 +190,9 @@ CWinThread::~CWinThread () void CWinThread::start () { + if (isRunning()) + throw EThread("Starting a thread that is already started, existing thread will continue running, this should not happen"); + // ThreadHandle = (void *) ::CreateThread (NULL, _StackSize, ProxyFunc, this, 0, (DWORD *)&ThreadId); ThreadHandle = (void *) ::CreateThread (NULL, 0, ProxyFunc, this, 0, (DWORD *)&ThreadId); // nldebug("NLMISC: thread %x started for runnable '%x'", typeid( Runnable ).name()); From afb32cfe1aec6b6eaf5b07a3c08a44030ca0df5e Mon Sep 17 00:00:00 2001 From: kaetemi Date: Wed, 11 Apr 2012 23:34:36 +0200 Subject: [PATCH 4/5] Fixed: Typos and warnings --- code/nel/include/nel/sound/group_controller.h | 2 +- code/nel/include/nel/sound/stream_file_sound.h | 2 +- code/nel/src/sound/audio_decoder_vorbis.cpp | 2 +- code/nel/src/sound/driver/CMakeLists.txt | 2 ++ code/nel/src/sound/group_controller.cpp | 3 ++- 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/code/nel/include/nel/sound/group_controller.h b/code/nel/include/nel/sound/group_controller.h index c3ac4838f..08d474129 100644 --- a/code/nel/include/nel/sound/group_controller.h +++ b/code/nel/include/nel/sound/group_controller.h @@ -52,7 +52,7 @@ namespace NLSOUND { class CGroupController : public UGroupController { public: - friend CGroupControllerRoot; + friend class CGroupControllerRoot; private: CGroupController *m_Parent; diff --git a/code/nel/include/nel/sound/stream_file_sound.h b/code/nel/include/nel/sound/stream_file_sound.h index 2429c3382..8c1cc1634 100644 --- a/code/nel/include/nel/sound/stream_file_sound.h +++ b/code/nel/include/nel/sound/stream_file_sound.h @@ -48,7 +48,7 @@ namespace NLSOUND { class CStreamFileSound : public CStreamSound { public: - friend CSourceMusicChannel; + friend class CSourceMusicChannel; public: CStreamFileSound(); diff --git a/code/nel/src/sound/audio_decoder_vorbis.cpp b/code/nel/src/sound/audio_decoder_vorbis.cpp index c54ce0ade..e0b950fc4 100644 --- a/code/nel/src/sound/audio_decoder_vorbis.cpp +++ b/code/nel/src/sound/audio_decoder_vorbis.cpp @@ -102,7 +102,7 @@ static ov_callbacks OV_CALLBACKS_NLMISC_STREAM = { }; CAudioDecoderVorbis::CAudioDecoderVorbis(NLMISC::IStream *stream, bool loop) -: _Stream(stream), _Loop(loop), _StreamSize(0), _IsMusicEnded(false) +: _Stream(stream), _Loop(loop), _IsMusicEnded(false), _StreamSize(0) { _StreamOffset = stream->getPos(); stream->seek(0, NLMISC::IStream::end); diff --git a/code/nel/src/sound/driver/CMakeLists.txt b/code/nel/src/sound/driver/CMakeLists.txt index d1d82169c..90fbbb562 100644 --- a/code/nel/src/sound/driver/CMakeLists.txt +++ b/code/nel/src/sound/driver/CMakeLists.txt @@ -3,6 +3,8 @@ FILE(GLOB HEADERS ../../../include/nel/sound/driver/*.h) NL_TARGET_LIB(nelsnd_lowlevel ${HEADERS} ${SRC}) +TARGET_LINK_LIBRARIES(nelsnd_lowlevel nelmisc) + SET_TARGET_PROPERTIES(nelsnd_lowlevel PROPERTIES LINK_INTERFACE_LIBRARIES "") NL_DEFAULT_PROPS(nelsnd_lowlevel "NeL, Library: Sound Lowlevel") NL_ADD_RUNTIME_FLAGS(nelsnd_lowlevel) diff --git a/code/nel/src/sound/group_controller.cpp b/code/nel/src/sound/group_controller.cpp index 1cce28bf0..fef67a01b 100644 --- a/code/nel/src/sound/group_controller.cpp +++ b/code/nel/src/sound/group_controller.cpp @@ -87,7 +87,8 @@ std::string CGroupController::getPath() // overridden by root return returnPath; } } - + nlerror("Group Controller not child of parent"); + return ""; } void CGroupController::calculateFinalGain() // overridden by root From df6213ed7906732613df8b9377bacd25abb85fa1 Mon Sep 17 00:00:00 2001 From: kaetemi Date: Thu, 12 Apr 2012 00:27:03 +0200 Subject: [PATCH 5/5] Fixed: #795 #1460 Linux compile of new sound samples --- .../samples/sound/stream_file/stream_file.cpp | 13 +++++++++++- .../stream_ogg_vorbis/stream_ogg_vorbis.cpp | 21 +++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/code/nel/samples/sound/stream_file/stream_file.cpp b/code/nel/samples/sound/stream_file/stream_file.cpp index 6ded512eb..14314c16d 100644 --- a/code/nel/samples/sound/stream_file/stream_file.cpp +++ b/code/nel/samples/sound/stream_file/stream_file.cpp @@ -18,7 +18,11 @@ // STL includes #include -#include +#ifdef NL_OS_WINDOWS +# include +#else +# include +#endif // NeL includes #include @@ -110,9 +114,16 @@ static void runSample() printf("Press ANY other key to exit\n"); for (; ; ) { +#ifdef NL_OS_WINDOWS if (_kbhit()) { switch (_getch()) +#else + char ch; + if (read(0, &ch, 1)) + { + switch (ch) +#endif { case '+': s_GroupController->setUserGain(s_GroupController->getUserGain() + 0.1f); diff --git a/code/nel/samples/sound/stream_ogg_vorbis/stream_ogg_vorbis.cpp b/code/nel/samples/sound/stream_ogg_vorbis/stream_ogg_vorbis.cpp index 2709a0b81..b3645913c 100644 --- a/code/nel/samples/sound/stream_ogg_vorbis/stream_ogg_vorbis.cpp +++ b/code/nel/samples/sound/stream_ogg_vorbis/stream_ogg_vorbis.cpp @@ -18,7 +18,11 @@ // STL includes #include -#include +#ifdef NL_OS_WINDOWS +# include +#else +# include +#endif // NeL includes #include @@ -138,9 +142,16 @@ static void runSample() printf("Press ANY other key to exit\n"); while (!s_AudioDecoder->isMusicEnded()) { +#ifdef NL_OS_WINDOWS if (_kbhit()) { switch (_getch()) +#else + char ch; + if (read(0, &ch, 1)) + { + switch (ch) +#endif { case '+': s_GroupController->setUserGain(s_GroupController->getUserGain() + 0.1f); @@ -172,7 +183,13 @@ static void runSample() printf("End of song\n"); printf("Press ANY key to exit\n"); - while (!_kbhit()) { s_AudioMixer->update(); nlSleep(10); } _getch(); +#ifdef NL_OS_WINDOWS + while (!_kbhit()) +#else + char ch; + while (!read(0, &ch, 1)) +#endif + { s_AudioMixer->update(); nlSleep(10); } return; }