?
Solved

self deleting class

Posted on 2007-10-14
11
Medium Priority
?
241 Views
Last Modified: 2010-08-05
Hi,

I'm trying to make a basic thread pool class (I've read some others):

class ThreadPool {
    list<Thread*> m_OpenThreads; // Thread pool.

     void Listen()
     {
         // kind of simulate getting requests for new threads.
         for (int i = 0; i < 5; i++) {
              EnterCriticalSection();
              if (m_OpenThreads.size() < MAX_THREADS) {
                  Thread* p = new Thread();
                  m_OpenThreads.push_back(p);
                  p->Run();
              }
              LeaveCriticalSection();
         }
      }

      // Remove the dead thread from the pool.
      void RemoveThread(Thread& t)
      {
          EnterCriticalSection();
          m_OpenThread.remove(t);
          LeaveCriticalSection();
};


class Thread {
      void Run(ThreadPool* pParent)
      {
            // end user must implement this to call their own thread handler function.
            DoYourThreadStuff();
           
            // Get ourselves out of the pool.
            pParent->RemoveThread(*this);
           
            // Delete ourselves...this is confusing...how can this work?
            delete *this;
        }

         virtual void DoYourThreadStuff() = 0;
}

This is just a rough idea. I tried it out and so far it worked (though I barely tested for real though). What I was most interested in is the fact that I could call delete *this in Thread::Run() without any problems. How is that possible? I mean the object has deleted itself but has yet to exit the function - I'm not clear on if that is legal or not.

Anyway, any comments would be great,

Thanks
0
Comment
Question by:DJ_AM_Juicebox
  • 5
  • 3
  • 2
  • +1
11 Comments
 
LVL 5

Expert Comment

by:abith
ID: 20076303
>>delete *this
this is legal. but deciding the use is upto the design of the class. its not safe to use in destructor. and after using this no member variable should be accessed.

>> ... the object has deleted itself but has yet to exit the function ...

member function is not associated with any object. its a common code specified for a class. so when you delete *this, it deletes only allocated memory(member variable's memory) for this object. so any member function  that doesnt use member variables after deleting *this, will complete its execution without any issues.
0
 
LVL 53

Expert Comment

by:Infinity08
ID: 20076417
I would be careful with this though, as you allocate the memory outside of the class :  

                  Thread* p = new Thread();

while you de-allocate inside the class :

                  delete *this;

How do you guarantee that it won't be allocated like this :

                  Thread t;

???
0
 
LVL 5

Expert Comment

by:abith
ID: 20076724
Agree!
forgot to mention in earlier post itself.
delete *this; will be working fine only if memory is created in heap.
0
Industry Leaders: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
LVL 39

Accepted Solution

by:
itsmeandnobodyelse earned 2000 total points
ID: 20077865
>>>>            // Delete ourselves...this is confusing...how can this work?
>>>>            delete *this;

The 'this' is a pointer like any other pointer. So whether 'delete this;' works or crashes only depends on whether the pointer was created on the heap (and wasn't already deleted) or not. After deletion the storage the 'this' was pointing to was freed, so any 'access' to that storage may fail if the storage was used by somewhat else in the meantime. In the above sample code the 'delete this' is the very last that was done in Thread::Run and returning to the caller wouldn't need access to the deleted storage. But, it only works cause
p->Run(); was one of the last statements of the calling function as well and the pointer p wasn't used after calling p-Run();

I would recommend to omit the delete this. You easily can do:

     void Listen()
     {
         // kind of simulate getting requests for new threads.
         for (int i = 0; i < 5; i++) {
              EnterCriticalSection();
              if (m_OpenThreads.size() < MAX_THREADS) {
                  Thread t;
                  m_OpenThreads.push_back(&t);
                  t.Run();
              }
              LeaveCriticalSection();
         }
      }

and the thread was deleted automatically.


0
 

Author Comment

by:DJ_AM_Juicebox
ID: 20078798
Yeah I know it's not a safe class, end users could go wrong with it easily. I just wanted to know really if calling delete this; inside its own member function was legal.

The problem with creating it on the stack like this:

    Thread t;
    m_OpenThreads.push_back(&t);
    t.Run();

is that people would have to edit that code to put in their own class type, which could make maintenance annoying I guess. I wanted to make it so that the end user just derives from Thread, and doesn't have to worry about any of the thread pool utility work. Is there some way to get around it with templates or something?

Thanks
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 20079086
>>>> Is there some way to get around it with templates or something?
Hardly. The template type must be known at compile time while you need something that works virtually on run-time. What you need is called 'factory' what creates derived class instances by name or by id.

    Thread* pt = theFactory.createThread(id);
    m_OpenThreads.push_back(pt);
    pt->Run();
    delete pt;  // delete virtually

theFactory is some singleton helper class which creates instances of the derived classes by using function pointers. The functions were 'registered' in a map at the factory. Each derived class needs to register a static create function like that:

  class DerivedThread : public Thread
  {
         static bool registered;
   public:
         static Thread* create() { return new DerivedThread(); }

         virtual ~DerivedThread();
         ...    
  };


// derivedthread.cpp

  bool DerivedThread::registered =
       theFactory.registerDerived(123, DerivedThread::create);

Instead of an integer id you may use the classname for key.

With the static bool registered you achieve one-time registration.

Regards, Alex










0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 20079170
Assume, you want to start one thread for each derived class. Then you could do

typedef Thread* (*CreateFunc)();
class Factory
{
     map<int, CreateFunc> factoryMap;
public:
     Thread* createThread(int id)
     {
           CreateFunc cf = factoryMap[id];
           if (cf != NULL)
                return cf();
           return NULL;
     }
     bool registerDerived(int id, CreateFunc cf)
     {
           factoryMap[id] = cf;
           return true;
     }
     int getDerivedList(vector<int>& ids)
     {
           map<int, CreateFunc>::iterator it;
           for (it = factoryMap.begin(); it != factoryMap.end(); ++it)
                ids.push_back(it->first);
           return ids.size();
     }
};

  vector<int> ids;
  int nthreads = theFactory.getDerivedList(ids);
  for (int i = 0; i < nthreads, ++i)
  {

    Thread* pt = theFactory.createThread(ids[i]);
    m_OpenThreads.push_back(pt);
    pt->Run();
    delete pt;  // delete virtually
  }

0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 20079227
Note, instead deriving from class Thread you may consider defining a class called Job and all user's were deriving from 'Job' rather from Thread. With that design a Thread was not derived but was part of the ThreadPool and was supposed to 'run' a job while being active. That means the ThreadPoolManager would create a fixed number of threads - say 64 - each of them was sleeping, e. g. by being blocked by waiting for an event. If a 'job' has to be done, an instance of the job-derived class was created (with or without a factory) and passed to the ThreadPoolManager. The ThreadPoolManager selects one free thread from queue (if none was free it waits fro a semaphore counting free jobs) and passes the 'job' to it. The thread processes the job by means of virtual calls and returns to ThreadPoolManager if done. And so on.

0
 

Author Comment

by:DJ_AM_Juicebox
ID: 20079232
itsmeandnobodyelse: I read through your post but I don't understand in the context of threads - in these lines:

    Thread* pt = theFactory.createThread(ids[i]);
    m_OpenThreads.push_back(pt);
    pt->Run();
    delete pt;  // delete virtually

Inside pt->Run() the derived class should handle actually creating the thread and running it concurrently - so Run() will pretty much return immediately back to the main thread. But then delete pt is called which will delete it while the thread function is still executing? I don't know if I'm getting confused with it -

Thanks

0
 

Author Comment

by:DJ_AM_Juicebox
ID: 20079335
ah well you guys answered my original question, don't want this to go too far into the thread pool stuff,

Thanks
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 20079576
>>>> But then delete pt is called which will delete it
>>>> while the thread function is still executing? I don't
>>>> know if I'm getting confused with it

You are right. But then 'delete this' is the wrong place to delete as well.

Note, as told in a 'thread pool' a thread should be able to process different things. If so, you never would terminate a thread (and therefore have to delete) until the application stops.

If creating asynchronous running threads you could/should pass a Thread* pointer to the thread function. If doing so and if  not storing a pointer to that Thread in some container, you can delete the Thread in the thread function immediately before returning.    

    Thread* pt = theFactory.createThread(ids[i]);
    m_OpenThreads.push_back(pt);
    pt->Run();

void Thread::Run()
{
        ThreadFunc tfunc = getThreadFunc();   // get function by virtual call
      _beginThread(tfunc, 0, this);  // pass this pointer to thread function
}

class DerivedThread : public Thread
{
     static void threadFunc(void* p)
     {
           Thread* pt = (Thread*)p;
           pt->Execute();   // do virtually what needs to be done

           delete pt;  // a wonderful place to delete as all was done
     }
};
0

Featured Post

What does it mean to be "Always On"?

Is your cloud always on? With an Always On cloud you won't have to worry about downtime for maintenance or software application code updates, ensuring that your bottom line isn't affected.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Errors will happen. It is a fact of life for the programmer. How and when errors are detected have a great impact on quality and cost of a product. It is better to detect errors at compile time, when possible and practical. Errors that make their wa…
Written by John Humphreys C++ Threading and the POSIX Library This article will cover the basic information that you need to know in order to make use of the POSIX threading library available for C and C++ on UNIX and most Linux systems.   [s…
The goal of the video will be to teach the user the difference and consequence of passing data by value vs passing data by reference in C++. An example of passing data by value as well as an example of passing data by reference will be be given. Bot…
The viewer will be introduced to the member functions push_back and pop_back of the vector class. The video will teach the difference between the two as well as how to use each one along with its functionality.

749 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question