Oxll

snoofle

In our system there is a singleton (that's used *everywhere*) called OrderExtractLinkedList. Naturally, at the top of almost every method we do: OrderExtractLinkedList oxll = OrderExtractLinkedList.getInstance(), and then use that locally. We also have hex constants all over the place (don't get me started).

Today, my boss comes over and asks me to try and track down why we are suddenly getting threads dying. The guy who wrote the code sits right next to my boss, but my boss apparently doesn't trust him a whole lot so he came to me.

After a LOT of code walk throughs, digging, debugging, and adding hundreds of print statements (there were 128 threads, so naturally it never executes in the same sequence twice - it was the only way to stay sane), I nailed it. Then I cried.

There was this class that was called way down at the bottom of the call hierarchy:

public class Yyy {
    public void someMethod(OrderExtractLinkedList oxll) { ... }
    public void someMethod(int x) { ... }
}

The way the code was used was that a deeply nested function call would be made to do stuff and update the database. If the data had been locked by another thread, we'd get a locking exception and would try again. The idea was that if we didn't get it after two tries, that something was really wrong and we should continue (an error flag would have been set by the underlying code).

Here's what happened deep down in the bowels of the system:

try {
     ref.someMethod(oxll);  // In case you can't tell: all letters
} catch (OurWrapperForADbLockingException e) {
     ref.someMethod(0x11);  // hex for 17
}

The coder meant to use the call that takes the int parameter. Thanks to auto pilot or auto complete, he inadvertently used the other one by entering "oxll" in the try-call. It failed every single time, and the catch-call always succeeded.

Until an actual locking issue arose. Then the second call failed, but because we continued on, some flag had been set which caused the code to enter a block which resulted in a null pointer exception which rolled off the stack and killed the thread.

This is how you do it people:

public void run() {

while (true) { // if your thread must run forever

try {

// your stuff here

} catch (Throwable t) { // fail safe

// deal with obviously miscoded uncaught error here

// by explaining that an unhandled-exception was caught

// and needs to be dealt with, and then dump the stack

}

}

}

Sutherlands

Aren't you not supposed to catch Throwable, and catch Exception instead?

Power_Troll

@Sutherlands said:

Aren't you not supposed to catch Throwable, and catch Exception instead?

Catching Throwable is pretty insane, but if your thread must run forever, such is life.

Sutherlands

JDK 22 Documentation - Home

Pretty sure most of those are deal-breakers.

snoofle

@Power Troll said:

@Sutherlands said:
Aren't you not supposed to catch Throwable, and catch Exception instead?

Catching Throwable is pretty insane, but if your thread must run forever, such is life.

Quite true, but these idiots not only intentionally throw RuntimeException (for you non-Java folks, only the jvm should do that), but they also were too lazy to subclass Exception in spots and directly throw Throwable (technically ok, but by convention a no-no).

This is the second place I've worked where I've seen folks do this. When you work with idiots, you have to make your code bullet proof, if only to deflect blame when their stuff breaks.

@snoofle said:

We also have hex constants all over the place

WITCH!

@snoofle said:

OurWrapperForADbLockingException

Yeah, I usually block all ads by default, but I create exceptions for sites I like to support.

msntfs

@snoofle said:

@Power Troll said:
@Sutherlands said:
Aren't you not supposed to catch Throwable, and catch Exception instead?

Catching Throwable is pretty insane, but if your thread must run forever, such is life.

Quite true, but these idiots not only intentionally throw RuntimeException (for you non-Java folks, only the jvm should do that), but they also were too lazy to subclass Exception in spots and directly throw Throwable (technically ok, but by convention a no-no).

This is the second place I've worked where I've seen folks do this. When you work with idiots, you have to make your code bullet proof, if only to deflect blame when their stuff breaks.

Where does the rule about only the JVM throwing RuntimeExceptions come from? IllegalArgumentException is a RuntimeException, for example and you certainly are supposed to throw it.
Moreover, I know a lot of projects that basicaly decided that JVM checked exceptions are just pain in the ass and throw RuntimeExceptions around. I see nothing fundamentally wrong with that

pjt33

@msntfs said:

Where does the rule about only the JVM throwing RuntimeExceptions come from? IllegalArgumentException is a RuntimeException, for example and you certainly are supposed to throw it.

The intended point, which I agree could have been expressed more clearly, appears to have been that application code should throw subclasses of RuntimeException rather than new RuntimeException().

PSWorx

@pjt33 said:

@msntfs said:
Where does the rule about only the JVM throwing RuntimeExceptions come from? IllegalArgumentException is a RuntimeException, for example and you certainly are supposed to throw it.

The intended point, which I agree could have been expressed more clearly, appears to have been that application code should throw subclasses of RuntimeException rather than new RuntimeException().

That doesn't change the fact that catch (Exception e) {} will catch raw RuntimeExceptions, too.

Now if your guys really start to subclass Throwable on the other hand...

mihi

A way to avoid catching Throwable, which is especially useful if you use custom ThreadGroups which have an unhandled exception handler that tries hard to delegate uncaught exceptions to the right place, and you do not want to duplicate that code:

public void run() {
    Runnable r = new RunnableThatShouldRunForever();
    while (true) {
        Thread t = new Thread(r);
        t.start();
        t.join();
    }
}

Yes, that will have one more thread waiting around and restart the thread on every exception (maybe add some delay to avoid spawning too fast?). But I guess you will have to bite one bullet...