Fixes for multithreaded smartpointers

[Benoit Germain]

Hello,

I am currently (and now successfully) using Loki on the Xbox 360. The API is very close to the Win32 API, so this was easy. To do so I had to tweak some include files, and also to fix a multithreaded smartpointer problem caused by the atomic functions critical section atomic_mutex_ not being properly initialized. Here is the list of changes:

LevelMutex.h:

+ #if defined( _XBOX)
+     #include <xtl.h>
+ #elif defined( _MSC_VER )
    #include <Windows.h>
#else
    #include <pthread.h>
#endif

Threads.h:

at header inclusion level insert:

+    #if defined(_XBOX)
+        #include <xtl.h>
+        #define LOKI_WINDOWS_H
+        #define LOKI_GLOBAL_NAMESPACE_PROLOGUE
+    #elif defined(_WIN32) || defined(_WIN64)
-    #if defined(_WIN32) || defined(_WIN64)
        #include <windows.h>
        #define LOKI_WINDOWS_H
+        #define LOKI_GLOBAL_NAMESPACE_PROLOGUE ::
    #else

inside #ifdef LOKI_WINDOWS_H (circa line 100) update the native call wrapper macros as follows:

- #define LOKI_THREADS_MUTEX_INIT(x)      ::InitializeCriticalSection (x)
- #define LOKI_THREADS_MUTEX_DELETE(x)    ::DeleteCriticalSection (x)
- #define LOKI_THREADS_MUTEX_LOCK(x)      ::EnterCriticalSection (x)
- #define LOKI_THREADS_MUTEX_UNLOCK(x)    ::LeaveCriticalSection (x)
+ #define LOKI_THREADS_MUTEX_INIT(x)      LOKI_GLOBAL_NAMESPACE_PROLOGUE InitializeCriticalSection (x)
+ #define LOKI_THREADS_MUTEX_DELETE(x)    LOKI_GLOBAL_NAMESPACE_PROLOGUE DeleteCriticalSection (x)
+ #define LOKI_THREADS_MUTEX_LOCK(x)      LOKI_GLOBAL_NAMESPACE_PROLOGUE EnterCriticalSection (x)
+ #define LOKI_THREADS_MUTEX_UNLOCK(x)    LOKI_GLOBAL_NAMESPACE_PROLOGUE LeaveCriticalSection (x)

then add this definition:

struct CRITICAL_SECTION_WRAPPER
{
private:
CRITICAL_SECTION m_cs;
public:
CRITICAL_SECTION_WRAPPER(void) { LOKI_THREADS_MUTEX_INIT(&m_cs); }
~CRITICAL_SECTION_WRAPPER(void) { LOKI_THREADS_MUTEX_DELETE(&m_cs); }
CRITICAL_SECTION *GetCS( void) { return &m_cs; }
};

then in the big LOKI_THREADS_ATOMIC_FUNCTIONS macro, change the following:

-        static CRITICAL_SECTION atomic_mutex_;   \
+        static CRITICAL_SECTION_WRAPPER atomic_mutex_;   \

and replace all &atomic_mutex_ arguments with atomic_mutex_.GetCS()

Near line 500, add the implementation of the atomic mutex as follows:

#ifdef LOKI_PTHREAD_H
    template <class Host, class MutexPolicy>
    pthread_mutex_t ObjectLevelLockable<Host, MutexPolicy>::atomic_mutex_ = PTHREAD_MUTEX_INITIALIZER;
+ #else
+     template <class Host, class MutexPolicy>
+     CRITICAL_SECTION_WRAPPER ObjectLevelLockable<Host, MutexPolicy>::atomic_mutex_;
+ #endif

and do the same near line 600:

#ifdef LOKI_PTHREAD_H
    template <class Host, class MutexPolicy>
    pthread_mutex_t ClassLevelLockable<Host, MutexPolicy>::atomic_mutex_ = PTHREAD_MUTEX_INITIALIZER;
+ #else
+     template <class Host, class MutexPolicy>
+     CRITICAL_SECTION_WRAPPER ClassLevelLockable<Host, MutexPolicy>::atomic_mutex_;
+ #endif

That's all :-)

[Richard Sposato]

Hi Benoit,

Thanks for posting this!  Your patch for getting Loki to work on the XBox looks promising.  I really applaud your efforts here.

I'm tempted to take a really deep look at adding your work to Loki so that other XBox programmers can use Loki.  However, before I do so, I need to ask a few questions.

Can you compile all other parts of Loki on the XBox?
Have you ported other parts of Loki to XBox?
If so, have you encountered any problems?
(I'm asking these questions because I'd really like to know how well Loki runs on other platforms besides Linux and Windows.)

Have you been able to compile Loki using your modified version of Thread.h using PThreads on either Windows or Linux?
How well does it compile on Windows (other than XBox) using CriticalSections?
If you did get it to compile, can you also compile the Loki test projects which exercise LevelMutex, smart-pointers, and threads?
(I ask these questions because Loki developers must make sure that Loki still compiles on Linux and Windows even when it gets ported to other platforms.)

Speaking of LevelMutex, I noticed you mentioned its header file in your post.  I hope you like that facility.  It's one of my favorite additions to Loki.

Thanks again for posting about your ability to compile and run Loki on the XBox.

Cheers,

Rich

[Benoit Germain]

Hello Rich,

Theses changes are the only ones I had to do to have the whole library build and run on Xbox 360. Unfortunately, at this point I am using it in a pure Xbox 360 prototype project for which no PC (either win32 or linux) version exists, so I can't tell much about it. The only parts I am using so far are the SmartPointer and Singleton. Some of the smartpointers use object level multithread lock. Integration is as follows, to be used inside a class declaration:

#define DECLARE_SINGLETON_TYPE(T) \
typedef Loki::SingletonHolder<T,Loki::CreateUsingNew,Loki::DefaultLifetime,Loki::ClassLevelLockable> Singleton; \
static inline T &Instance() { return Singleton::Instance(); }

#define DECLARE_POINTER_TYPES(T) \
static T *Null( void) { return 0x0; } \
typedef Loki::SmartPtr<T,Loki::RefCountedMTAdj<Loki::ObjectLevelLockable>::RefCountedMT> HardPtrMT; \
typedef Loki::SmartPtr<T const,Loki::RefCountedMTAdj<Loki::ObjectLevelLockable>::RefCountedMT> ConstHardPtrMT; \
typedef Loki::SmartPtr<T> HardPtr; \
typedef Loki::SmartPtr<T const> ConstHardPtr

Building Loki with the provided Loki_MSVC_8.sln yields a few compilation warnings that seem unrelated to my changes:

loki-0.1.7\include\loki/SPCachedFactory.h(173) : warning C4355: 'this' : used in base member initializer list
loki-0.1.7\test\regressiontest\assocvectortest.h(299) : warning C4996: 'sprintf': This function or variable may be unsafe. Consider using sprintf_s instead.
loki-0.1.7\include\loki\flex\smallstringopt.h(320) : warning C4244: '-=' : conversion from 'int' to 'char', possible loss of data
loki-0.1.7\include\loki/SmartPtr.h(202) : warning C4150: deletion of pointer to incomplete type 'Thingy'; no destructor called

And that's it. I don't have a linux/pthread box handy so I can't tell for this part of the code.

I rely a lot on Loki::Mutex for thread synchronisation, so that I can keep a minimum level of portability to the code (the prototype should make it to the PS3 too in a distant future). What I missed was condition variables. I had to grab an implementation from another place in order to code an efficient task scheduler. Maybe these could make it to the library too?

Regards,

Benoit.

[Benoit Germain]

Hello,

One more little thing: console developers tend to disable RTTI and exception handling because of the memory and performance hits they incur. Therefore, I get immediate compilation warnings regarding try/catch/throw code, such as:

loki-0.1.7\src\SmallObj.cpp(803) : warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc

It would be nice to encapsulate exception handling inside some macros that users could enable or disable at will. After all, errors are not supposed to happen in shipped games :-)

I also get a warning here

loki-0.1.7\include\loki\singleton.h(819) : warning C4702: unreachable code.

    void SingletonHolder<T, CreationPolicy,
        LifetimePolicy, ThreadingModel, MutexPolicy>::MakeInstance()
    {
        typename ThreadingModel<SingletonHolder,MutexPolicy>::Lock guard;
        (void)guard;
       
        if (!pInstance_)
        {
            if (destroyed_)
            {
                LifetimePolicy<T>::OnDeadReference();
                destroyed_ = false;
            }
            pInstance_ = CreationPolicy<T>::Create();
            LifetimePolicy<T>::ScheduleDestruction(pInstance_,
                &DestroySingleton);
        }
    }

because                 LifetimePolicy<T>::OnDeadReference();

is implemented as

                static void OnDeadReference()
                {
                    throw std::logic_error("Dead Reference Detected");
                }

which the compiler detects as not being able to execute further code.

Regards,

Benoit.