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

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 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 #include #include #include

namespace myproject { namespace trace {

//     template <typename T> //     struct storage_for //     { //       boost::aligned_storage 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 #include #include #include #include #include

#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_ptrboost::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 #include #include

#include <sstream>

#ifndef WIN32 #include #else #include #endif /* WIN32 */

namespace myproject { namespace trace {

   Trace::Trace ()      : m_hasWork ( true ),        m_state ( "Trace", "Trace" )    {      // start thread      m_thread = boost::shared_ptrboost::thread ( new boost::thread ( boost::bind ( &Trace::threadMain, this ) ) );    }

   Trace::~Trace()    {      // stop thread      {        boost::lock_guardboost::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_guardboost::mutex lock ( m_lock );            m_queue.push_back ( trace->clone() );          }

         m_condition.notify_one();    }

   void Trace::threadMain()    {      boost::unique_lockboost::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 #include

#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@lists.boost.org http://lists.boost.org/mailman/listinfo.cgi/boost-users