• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

does this fail to close the connection object

 
Bartender
Posts: 1810
28
jQuery Netbeans IDE Eclipse IDE Firefox Browser MySQL Database Chrome Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Maybe this belongs in another forum; sorry if I picked the wrong one. This is a DAO. This is the original code that FindBugs reported may fail to close database objects.



I see lots of problems with this code but most important is the lack of a finally block. I won't even go into the use of Vector, hard coded database credentials (removed to protect the guilty) and concatenating Strings inside a loop. My main concern is leaving database resources open. I'll fix the other issues later. For now, I changed it to this:



This is modeled after other DAOs I've written which FindBugs doesn't complain about. However, Findbugs is telling me that this code may still fail to close a database resource. It looks to me like the finally block should close everything no matter how the method exits. What am I missing?

 
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Yes, that code is fine. The only way one of those close() calls wouldn't be reached is if an Error is thrown in a previous close(), like OutOfMemoryError. You could catch Throwable instead of Exception inside the finally to handle that case, but I wouldn't advise that. If a RuntimeException is thrown, that's one thing. A coding error inside a close() shouldn't prevent a later close(). But an Error is a different story. That's more severe, usually indicative of a serious problem inside the JVM. Generally best just to let those bubble all the way up and kill the JVM. And if you do catch Error, you'd have to rethrow ThreadDeath.

It might be interesting to catch Error just to see if that shuts findbugs up, or if it's just confused about something else.

Finally, note that if you're in Java 7, you can use "with resources" or something like that to automatically close stuff that you explicitly mention at the start of the "with" block. Not sure of the details, but you an google for it if that's an option for you.
 
J. Kevin Robbins
Bartender
Posts: 1810
28
jQuery Netbeans IDE Eclipse IDE Firefox Browser MySQL Database Chrome Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks for checking it for me. I did a clean and build and recreated the findbugs project and it's no longer complaining. It's a useful tool, but it can be rather quirky.

Interesting tactic, catching Error or Throwable instead of Exception. I'll keep that one in mind in the future. I realize it's not a good idea for production code but it could be very useful for debugging.
 
Bartender
Posts: 2292
3
Eclipse IDE Spring Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Just to keep the forum well organized, let's move this thread to the JDBC forum, since it has more to do with JDBC.
 
reply
    Bookmark Topic Watch Topic
  • New Topic