views:

936

answers:

8

in my code I'm using

Thread.currentThread().sleep(sleepTime);

in the main (non Thread object) portion of the code.

It appears to be working fine, but I'm worried that there might be some hidden pitfall that will bite me in the ass later.

Is there a better way to make your main process sit around for a while? or is this the prescribed methodology?

EDIT:

In answer to why I'm doing it this way...

I have a process that connects via HTTP or FTP to remote hosts and does stuff.

In other words...

stuff...

connect to remote...

do stuff with remote connection...

close connection...

more stuff...

repeat as necessary.

I've found that in very very rare instances, the connection just goes off into la la land. It doesn't fail, it doesn't throw any exception, it just goes away. And it's blocking, so there's no in-line way to set a timer.

So, my solution is to do this...

stuff...

start new thread with connection in it...

go into an infinite loop with a timer in the MAIN process (not in the spawned thread) and wait for either

a) the connection thread to complete its task and set some flag to "done"

or

b) wait some preset amount of time and if the connection thread has not reported that it's finished, kill it and move on.

It is in the main process that I intend to sleep for some time, wake up, and see if the MAX_WAIT_TIME has expired. If not, go back to sleep and wait some more.

It seems much more efficient (on the processor) than sitting in standard while loop, since that would really interfere with the connection thread's business of doing what it needs to do.

I guess my question really is... is there anything unsafe about this approach. I see from the answers that, given what I'm doing, it looks like there isn't. Perhaps I should have asked if there is a more standardized approach?

+4  A: 

It's not dangerous, but in 99+% of cases when you think you need it, you really don't. What are you trying to do?

erikkallen
I would say only 95+% of case. ;)
Peter Lawrey
+3  A: 

There's no pitfall. It will sleep for as long as you tell it to.

There may or may not be a pitfall in the idea of your application falling asleep for a prolonged period of time.

mquander
Well, it will not always sleep as long as you tell it to. It can be interrupted by another thread.
Bombe
+3  A: 

Well, Thread.sleep is a static method so it is very misleading. Also you can't be woken up cleanly (you can interrupt, but I'd dispute that that is clean) in case you need the action to shutdown.

Tom Hawtin - tackline
+12  A: 

What kind of application are you writing? It's rarely a good idea, and it's a particularly bad idea if you're writing a client GUI - while the thread is sleeping, it won't be responsive.

If you could give more of an indication why you need to pause and the kind of application you're writing, that would help.

One other thing - your call should really be:

Thread.sleep(sleepTime);

Calling it via currentThread() makes it look like it's an instance method, but it's not - it's just a normal static method. You can't make any other thread sleep.

You should see if your IDE has an option to make calling static methods via a reference into a warning or error - it leads to misleading code (like this).

Jon Skeet
+2  A: 

If you decide to use Thread.sleep, ensure that you handle the InterruptedException appropriately.

McDowell
Interesting link(+1). I HATE that sleep throws a checked exception, and now I hate it more, but at least I know that maybe there are some cases where I should at least acknowledge it (restore interrupted status). Still I've never seen it cause a problem in sleeps that don't expect an interrupt.
Bill K
I call Thread.currentThread().interrupt() in my catch blocks out of habit. Maybe it's not a good thing to do out of habit, but (I think) it beats ignoring the exception.
Michael Myers
A: 

What do you mean the connection just "goes away"? Sure there's no inline way to set a timer, but you can set a connect timeout AND read timeouts if you want.

Create the socket with the no-args constructor, the call connect(SocketAddress, int) so that you can set a timeout (in milliseconds). If the connection can't be established in that time, an exception is thrown.

And you can call setSoTimeout() before connecting so that any calls to read() will only block for the amount of time you specify, instead of forever. If data can't be read in the time you specified, an exception is thrown.

Chochos
I thought of that, but I'm not sure exactly where the process "goes away" (and it literally just goes away, I have no other way to describe it. It sits for ever and reports nothing). My timer is a catch-all, whereas your approach only catches the connection and the read. Is there a reason...
Dr.Dredel
that the timer is a bad idea?
Dr.Dredel
A: 

AFAIR Thread.sleep() wastes CPU time while Object.wait(long timeout) does not. So you should always use Object.wait(long timeout) instead. Although I can't find any footage that supports my thesis, I reckon that when switching to Object.wait(long timeout) calls we received a large performance gain.

dhiller
A: 

People often use Timer to perform delayed events but I prefer the ScheduleExecutorService. You can use the same thread pool for all your timeout actions. (You can have a thread pool of size 1)

Peter Lawrey