The Daily WTF: Curious Perversions in Information Technology

2014-01-14 Reply Admin

// Assume everything is initialized

2014-01-14 Reply Admin

Meep:
Dan the man:

So..... if the thread is being shut down, surely the server is being shut down? Why do you want to log the fact that it happened to be sleeping at the exact moment when the server was being shut down. What do you gain from that?

There is no reason to assume that a thread is only shut down when the entire process is terminating.

It only means a certain task is being interrupted. For instance, if I'm doing a file transfer and want the user to be able to click "Cancel," I want to stop the transfer promptly. There's no reason to assume the entire application is shutting down.

Similarly, Tomcat has the ability to load a new version of a web app in place. What it does is it loads the new version and starts handing new requests to the new version. When the old version has no more active requests, it is directed to shut down. (It's pretty slick when it works.)

So a web app in Tomcat needs to release any threads its using in a timely fashion, but it explicitly does not mean that the entire process is terminating.

In most cases, instead of swallowing the exception, you just return or throw some kind of unchecked exception. Or, if you can't handle the interrupt immediately, you use Thread.interrupt() to set the thread's interrupt flag and the next block that can throw an InterruptedException will throw immediately.

Either way, I'm genuinely interested whether anyone can actually improve on this code.

Really, you think it makes sense to hammer the database every 10 ms until the number of rows has actually increased?

.....

Whooooooaaa there. I see how you could jump to the conclusion that I was talking about all the code, but I was actually referring to the snippet in the current thread (which you can't see because of the wonderful messageboard software):

             try {

                      Thread.sleep(10); // 10 milliseconds

                  } catch (InterruptedException ie) {

                    // do nothing

                  }

This WTF as a whole definitely deserves its title 150% - it made my jaw drop. I get your point about Tomcat deployment unloading - I haven't worked with Tomcat for a while unfortunately.

But I would like to learn something from this..... so lets assume, for a moment, that the sleeping is waiting on something that is guaranteed to complete in a timely fashion.... and for some reason we needed to wait for it. Wouldn't this be an ok way of handling it, if we threw an unchecked exception?

Or would it be much better to use some kind of Join functionality with the thread (assuming we started the thread ourselves)?

2014-01-14 Reply Admin

Dan the man:
I was actually referring to the snippet in the current thread (which you can't see because of the wonderful messageboard software):
             try {
                  Thread.sleep(10); // 10 milliseconds

              } catch (InterruptedException ie) {

                // do nothing

              }
This WTF as a whole definitely deserves its title 150% - it made my jaw drop. [...] But I would like to learn something from this..... so lets assume, for a moment, that the sleeping is waiting on something that is guaranteed to complete in a timely fashion.... and for some reason we needed to wait for it. Wouldn't this be an ok way of handling it, if we threw an unchecked exception? Or would it be much better to use some kind of Join functionality with the thread (assuming we started the thread ourselves)?

If you look at the code, you will see that InboundMessageHandler in a while loop starts a thread to insert some msg into a database.

After starting the db-worker-thread (if I may call it that), InboundMessageHandler sleeps until the operation has succeeded, i.e. the message has been added to the database.

This effectively means that only one db-worker-thread per InboundMessageHandler instance is ever running.

InboundMessageHandler does not feature any kind of notification system, i.e. db-worker-thread does not notify the InboundMessageHandler instance that it has succeeded. Therefore, it's up to the InboundMessageHandler instance to find out if this is the case.

To achieve this, InboundMessageHandler sleeps for 10ms, looks at the database to see if rowcount has increased and if not sleeps again for 10ms, looks again, etc.

Yes, this is bad and the main WTF of the article.

But it also means it is ok to ignore the InterruptedException, since it only means that the InboundMessageHandler instance will in one round get less than 10ms of sleep. It wakes up, yawns, looks at the alarm clock, shakes its head, checks if rowcount has increased, and if not, goes back to sleep.

There are cases when a "do nothing" is ok, this is one of them. Good reading in this case is actually http://www.ibm.com/developerworks/java/library/j-jtp05236/index.html: the sentence "nor reassert the thread's interrupted status" is the key, since this is exactly what InterruptedException does. It looks around, finds the situation to be unchanged, and goes back to the state before the InterruptedException was caught.

So, although the code is quite a big WTF, the "//do nothing" is completely ok.

2014-01-14 Reply Admin

Well that's stupid and pointless.

2014-01-15 Reply Admin

faoileag:
There are cases when a "do nothing" is ok, this is one of them. Good reading in this case is actually http://www.ibm.com/developerworks/java/library/j-jtp05236/index.html: the sentence "nor reassert the thread's interrupted status" is the key, since this is exactly what InterruptedException does. It looks around, finds the situation to be unchanged, and goes back to the state before the InterruptedException was caught.
So, although the code is quite a big WTF, the "//do nothing" is completely ok.

It isn't and your link actually say exactly the opposite of what you claim:

The worst thing you can do with InterruptedException is swallow it -- catch it and neither rethrow it nor reassert the thread's interrupted status.

The most applicable example for this case would be:

public Task getNextTask(BlockingQueue<Task> queue) { boolean interrupted = false; try { while (true) { try { return queue.take(); } catch (InterruptedException e) { interrupted = true; // fall through and retry } } } finally { if (interrupted) Thread.currentThread().interrupt(); } }

2014-01-15 Reply Admin

It doesn't crash so it's fine, right? lol

2014-01-15 Reply Admin

I wonder what would happen if the transaction deleted (or updated) rows, rather than inserting them?

chubertdev · 2014-01-15 Reply Admin

jahcsoft:
It doesn't crash so it's fine, right? lol

Yup, it's not like exceptions are expensive or anything.

2014-01-16 Reply Admin

TRWTF is JDBC do not have async APIs.

2014-01-24 Reply Admin

In most cases, instead of swallowing the exception, you just return or throw some kind of unchecked exception. Or, if you can't handle the interrupt immediately, you use Thread.interrupt() to set the thread's interrupt flag and the next block that can throw an InterruptedException will throw immediately.

Yes!

I'd like to add that a good way of generally handling unexpected (or impossible) InterruptedExceptions is to do this:

try {
  sleep(...) // or whatever
} catch (InterruptedException exception) {
  // Restore interrupted exception since
  // we are not really "handling" it here
  Thread.currentThread().interrupt();
  
  // We aren't expecting anyone to actually
  // cancel us so turn it into an illegal state
  throw new IllegalStateException(
    "Unexpected InterruptedException.",
    exception);
}

This pattern has a few advantages..

The irritating aspects of the checked exception go away
The IllegalStateException documents that you're not seriously expecting to be cancelled here - you're just dealing with the exception
If someone screws up a refactoring and actually does cancel the thread (aka interrupt) an IllegalStateException will be throw which will, in fact, most likely kill the thread (and the exception will show up in the log.. hopefully someone will then fix the code)
If someone re-uses the code and doesn't notice this stuff and is catching "Exception" or "Throwable" (bad practice in the general sense but it happens quite a bit in worker type threads) then the interrupt flag is still set and top level code checks for isInterrupted() (or futher sleep()) will still work properly.

Cheers guys.

2016-07-27 Reply Admin

As volume rose, the number of threads that got spawned exceeded the capacity of the system

Reminds me of something I overheard a student say at my university:

Perhaps we should be using a bounded number of threads

Throttling Throughput

Leave a comment on “Throttling Throughput”