Re: [Boost-users] Adding a thread to a Singleton class

Subject: Re: [Boost-users] Adding a thread to a Singleton class
From: Gottlob Frege (gottlobfrege_at_[hidden])
Date: 2010-01-12 01:09:40

Replying from an iPhone so excuse my terseness and mistakes.

> Since this is a library I do not believe it is called before main but I
> cannot give you a reason why believe that. So since its a conjecture we
> cannot go on that. Is there a way to do a trace of the code as it runs
> to see if the constructor is called before main?
>

you could set a breakpoint inside your constructor and see where it is
called from. Alternatively you could log something from main and then
see if it is the first thing in the log file or if something came
before it.

But in general the question is whether or not anyone will ever call
logging before main. I'd like logging to always be available so if I
need to use it with some static class, then I'd like that option. In
particular imagine someone writing an allocator not an easy task. They
want to use logging to help them debug their allocator. And there is a
good chance there  allocator is called before main (ie if it is a
system wide replacement for 'new' not just an allocator for vector or
something).  So it would be nice to be able to log inside of
constrictors of global statics called before main. Or you could defer
that problem until someone actually needs it.   Your logger allocates
memory anyhow (via strings etc) so supporting logging from within the
allocator would be quite a tricky bit of work. Something to think
about in the future however.

P.S. There is a logging library proposed for boost I think so maybe
someday all of these problems will go away.

>
>>>

>>
>> template <typename T>
>> struct storage_for
>> {
>>    aligned_storage<sizeof(MyCriticalSection),
>>                    alignment_of<MyCriticalSection>::value> data;
>> };
>>
>>           storage_for<T> storage;
>>
>> then in place new:
>>
>>           m_obj.reset( new(&storage) T() );
>
> Where is "MyCriticalSection" declared? Is that my internal class called
> Trace_State which actually does the logging?
>

sorry. That was just a cut and paste error on my part. It is suppose to be T.

>> Does this 'work' or do you need to know T?
>
> I am not sure what you are asking here?
>

the following 2 lines declaring instances of m_obj and flag, but
generic instances with T not specific instances with a specific type
in place of T. Maybe my compiler just doesn't support that but I
thought you could only instantiate specific types.

>> > template <typename T> boost::scoped_ptr<T> myproject::trace::Singleton<T>::m_obj ( 0 );
>> > template <typename T> boost::once_flag myproject::trace::Singleton<T>::m_flag = BOOST_ONCE_INIT;
>> >
>> >

>
> Ok. I am used to multi-threaded programming in C# but not sure if I had
> the semantics right here. What I was hoping to happen was:
>
>         WHILE ( there is work to be done )
>           {
>                 // I have work to do
>                 WHILE ( input queue is empty )
>                  {
>                         // wait for work
>                  }
>
>                  // process the work to be done
>            }
>
> Is that what I have done?

Yep just be aware that you have the lock. Usually that's what is
wanted but obviously in you case you want to unlock after copying but
before processing.

>
> You are right. It is locked because we are still in the scope of
> threadMain. Now this will cause a problem. I wanted m_queue to be open
> for writing after making a copy while the internal thread handled
> writing the contents of its copied queue to disk. Is there a better way
> to write threadMain and processTraces?
>
>

I think you should copy the queue in threadMain then unlock then pass
the local copy to processTraces. Then relock before looping around.
I would NOT unlock/relock -inside- processTraces.  I think it would
beuch clearer to keep all locking inside threadMain.

> So change addTrace to something like after adding a clone method:
>

yep.
Note that it is up to you to decide whether the protocol should be 'i
will clone whatever you send to me' vs 'i will take ownership of
whatever you send to me'. Either can work as long as your interface is
clear and enforces your protocol.

>
> I appreciate the comments. I will keep appending this email with the
> updated code so to further help our discussion. Is there a preferred way
> to attach code?

No idea.

> I was thinking that just adding it as an attachment
> would not allow you to easily make comments.
>
> Stephen
>
> ------------------------- Singleton_T.h -------------------------------
> #ifndef SINGLETON_H
> #define SINGELTON_H
>
> #include <boost/utility.hpp>
> #include <boost/thread/once.hpp>
> #include <boost/scoped_ptr.hpp>
> #include <boost/type_traits/aligned_storage.hpp>
>
> namespace myproject { namespace trace {
>
> //     template <typename T>
> //     struct storage_for
> //     {
> //       boost::aligned_storage<sizeof(T), boost::alignment_of<T>::value> data;
> //     };
>
>     template <typename T>
>     class Singleton : boost::noncopyable
>     {
>     public:
>
>       static T& Instance()
>       {
>         boost::call_once ( init, m_flag );
>         return *m_obj;
>       }
>
>       static void init ()
>       {
>         //m_obj.reset ( new (&storage) T() );
>         m_obj.reset ( new T () );
>       }
>
>     protected:
>
>       ~Singleton(){}
>       Singleton(){}
>
>     private:
>
>       static boost::scoped_ptr<T> m_obj;

of you use storage_for then you need a scoped_ptr that doesn't delete
the memory on exit.

>       static boost::once_flag m_flag;
>       //storage_for<T> storage;
>     };
>
>   } // namespace trace
> } // namespace trace
>
> template <typename T> boost::scoped_ptr<T> myproject::trace::Singleton<T>::m_obj ( 0 );
> template <typename T> boost::once_flag myproject::trace::Singleton<T>::m_flag = BOOST_ONCE_INIT;
>
> #endif // define SINGELTON_H
>
> ---------------------- Trace.h -----------------------
> #ifndef TRACE_H
> #define TRACE_H
>
> #include <boost/cstdint.hpp>
> #include <boost/shared_ptr.hpp>
> #include <boost/thread/mutex.hpp>
> #include <boost/thread/condition_variable.hpp>
> #include <boost/thread/thread.hpp>
> #include <boost/utility.hpp>
>
> #include <vector>
>
> #include "NumberTraceable_T.h"
> #include "Singleton_T.h"
> #include "StringTraceable.h"
> #include "Trace_State.h"
>
> namespace myproject { namespace trace {
>
>     //
>     // Turn Trace into a singleton class that contains a worker thread
>     // - Wait for data
>     //   - If data exists then process
>     //   - Else wait
>     // Add method for adding a ITraceable object. This will lock the logging vector, add
>     //  element then signal the boost condition variable
>     class Trace : public Singleton<Trace> {
>
>       friend class Singleton<Trace>;
>
>     public:
>
>       void add_Trace ( interface::ITraceable::ptr_t trace );
>
>     private:
>
>       Trace();
>
>       // Worker thread functions
>       void threadMain();
>       void processTraces();
>
>       // Synchronization variables
>       boost::mutex m_lock;
>       boost::condition_variable m_condition;
>
>       // Data variables
>       typedef std::vector < myproject::interface::ITraceable::ptr_t > QueueType;
>       QueueType m_queue;
>
>       // Thread variables
>       boost::shared_ptr<boost::thread> m_thread;
>       volatile bool m_hasWork;

by the way volatile is useful for threads in C# but not really in c++.
You can just get rid of that. You now always set and get the bool
within a lock, which is the correct way.

>
>       // Logging
>       Trace_State m_state;
>     };
>
>   } /* namespace trace */
> } /* namespace myproject */
>
> #endif /* TRACE_H */
>
> --------------------- Trace.cpp -------------------------------
> #include "Trace.h"
> #include "myproject/errors/Internal_Exception.h"
>
> #include <boost/bind.hpp>
> #include <boost/format.hpp>
> #include <boost/date_time/posix_time/posix_time.hpp>
>
> #include <sstream>
>
> #ifndef WIN32
> #include <unistd.h>
> #else
> #include <windows.h>
> #endif /* WIN32 */
>
> namespace myproject { namespace trace {
>
>     Trace::Trace ()
>       : m_hasWork ( true ),
>         m_state ( "Trace", "Trace" )
>     {
>       // start thread
>       m_thread = boost::shared_ptr<boost::thread> ( new boost::thread ( boost::bind ( &Trace::threadMain, this ) ) );
>     }
>
>     Trace::~Trace()
>     {
>       // stop thread
>       {
>         boost::lock_guard<boost::mutex> lock ( m_lock );
>         m_hasWork = false;
>       }
>
>       m_condition.notify_one();
>
>       m_thread.join();
>     }
>
>     void Trace::add_Trace ( interface::ITraceable::ptr_t trace )
>     {
>           // lock queue
>           {
>             boost::lock_guard<boost::mutex> lock ( m_lock );
>             m_queue.push_back ( trace->clone() );
>           }
>
>           m_condition.notify_one();
>     }
>
>     void Trace::threadMain()
>     {
>       boost::unique_lock<boost::mutex> lock ( m_lock );
>
>       while ( m_hasWork )
>         {
>           while ( m_queue.size() == 0 )
>             {
>               m_condition.wait ( lock );
>             }
>
>           this->processTraces();
>         }
>     }
>
>     void Trace::processTraces()
>     {
>       QueueType local_queue;
>
>       std::copy ( m_queue.begin(), m_queue.end(), local_queue.begin() );
>       m_queue.clear();
>
>       for ( QueueType::iterator pos = local_queue.begin();
>             pos != local_queue.end();
>             pos++ )
>         {
>           m_state.write_Message ( (*pos)->get_Level(), (*pos) );
>         }
>     }
>
>   } /* namespace trace */
> } /* namespace myproject */
>
> ------------------ ITraceable.h ---------------------------
> #ifndef ITRACEABLE_H
> #define ITRACEABLE_H
>
> #include <boost/cstdint.hpp>
> #include <boost/shared_ptr.hpp>
>
> #include <string>
>
> namespace myproject { namespace  interface {
>
>     class ITraceable {
>
>     public:
>
>       typedef boost::shared_ptr<ITraceable> ptr_t;
>
>       virtual ITraceable::ptr_t clone () = 0;
>
>       virtual std::string get_Formatted_String ( void ) = 0;
>     };
>
>   } // namespace trace
> } // namespace myproject
>
> #endif // ITRACEABLE_H
>
>
> _______________________________________________
> Boost-users mailing list
> Boost-users_at_[hidden]
> http://lists.boost.org/mailman/listinfo.cgi/boost-users
>