views:

62

answers:

4

Is it safe to start a thread pool in an objects constructor? I know that you shouldn't start a thread from a constructor, something about the "this" pointer escaping (I don't exactly understand this, but will do some more searches to try and figure it out).

The code would look something like this:

private ExecutorService pool;

public handler()
{
  pool = Executors.newCachedThreadPool();
}

public void queueInstructionSet(InstructionSet set)
{
  pool.submit(new Runnable that handles this instruction set);
}

If that doesn't work, i could just create this class as a Runnable and start it in a new thread. However, that seems like it would be adding an unnecessary thread to the program where it doesn't really need one.

Thanks.

EDIT:

Thanks for the replies everyone, they definitely helped make sense of this.

As per the code, in my mind it makes sense that this constructor creates the thread pool, but let me explain what specifically this code is doing, because i may be thinking about this in a weird way.

The entire point of this object is to take "Instruction Sets" objects, and act on them accordingly. The instruction sets come from clients connected to a server. Once a full instruction set is received from a client, that instruction set is sent to this object (handler) for processing.

This handler object contains a reference to every object that an instruction set can act upon. It will submit the instruction set to a thread pool, which will find which object this instruction set wants to interact with, and then handle the instruction set on that object.

I could handle the instruction set object in the IO server, but my thoughts are having a separate class for it makes the entire code more readable, as each class is focusing on doing only one specific thing.

Thoughts? Advice?

Thanks

+2  A: 

Your sample code doesn't let "this" escape at all. It's reasonably safe to start a new thread in a constructor (and even use this as the Runnable, which you don't in this example) so long as you're sure that you've already initialized the object as far as the new thread will need it. For example, setting a final field which the new thread will rely on after starting the thread would be a really bad idea :)

Basically letting the "this" reference escape is generally nasty, but not universally so. There are situations in which it's safe. Just be careful.

Having said that, making a constructor start a thread might be seen as doing too much within the constructor. It's hard to say whether it's appropriate in this case or not - we don't know enough about what your code is doing.

EDIT: Yup, having read the extra information, I think this is okay. You should probably have a method to shut down the thread pool as well.

Jon Skeet
Thanks, that helps. I updated the OP with what this code is specifically doing. I think it makes sense to have the constructor create the thread pool, but I am fairly new to this and would welcome any advice i can get :)
kyeana
@kyena: Okay, edited answer. Basically it looks like it's probably okay. Your class is effectively *being* a thread pool (via composition).
Jon Skeet
@Jon Skeet Thanks! I appreciate the help :)
kyeana
+1  A: 

I agree with Jon.

Furthermore, let me point that you're not actually starting any actions on the thread pool in the constructor. You're instantiating the thread pool, but it has no tasks to run at that point. Therefore, as written, you're not going to have something start operating on this instance before it finishes construction.

RonU
+1  A: 

It sounds like the thread pool would be owned and used by the object; threads wouldn't be pass out of the object. If that's the case, it shouldn't be an issue.

Constructors create an object and initialize its state. I can't imagine a use case where long-running processes are required to do so.

I can see where an object might interact with a thread pool to accomplish a task, but I don't see the need for that object to own the thread pool.

More details might help.

duffymo
Thanks, that helps. I updated the OP with what this code is specifically doing regarding the thread pool. I think it makes sense to have this object own the thread pool, but I am fairly new to this and would welcome any advice i can get :)
kyeana
A: 

I think it's OK to start a thread pool in the constructor of the object as long as that object fully manages the lifetime of that thread pool.

If you go this path, you will have to work extra hard to provide the following guarantees:

  1. If you constructor throws any exception ( both Runtime and checked ), you must have cleanup code in the constructor that shuts down the thread pool. If you don't do this and create a thread pool with non-daemon threads then, for example, a little console program that uses your object may stay up forever, leaking valuable system resources.
  2. You need to provide something that I call destructor method, similar to close in Java I/O. I usually call it releaseResources. Notice that finalize is not a substitute for this method, because it is called by GC, and for an object with reasonably small memory footprint it may never be called.
  3. When using this object follow this pattern

->

MyThreadPoolContainer container =
    new MyThreadPoolContainer( ... args to initialize the object... );

try
{
  methodThatUsesContainer( container );
}
finally
{
  container.releaseResources( );
}
  1. Document that object constructor allocates limited resources and the destructor method has to be called explicitly to prevent their leakage.
Alexander Pogrebnyak