Searching for missing things. How could it be done?

Hi there,

I have the following problem: I attempt to find the places in the codebase
where the jdbc connections are not closed properly (for instance). So I'm
looking for pieces of code that open a connection and don't close it in the
same method.

Generally speaking, I look for code fragments that have a certain thing and
don't have another in a given scope.

Two things come to mind: inspections and structural search. Having an inspection

for this type of thing would be way cool (not sure how hard to implement
it would be, though), but I was hoping I could use the structural search
for now. Didn't manage to figure out a solution, so far. Can it be done?

Thx,
Andrei



17 comments

Andrei Oprea wrote:

Hi there,

I have the following problem: I attempt to find the places in the
codebase where the jdbc connections are not closed properly (for
instance). So I'm looking for pieces of code that open a connection and
don't close it in the same method.


It seems the Simple Monster had/has some plans for this:
http://intellij.net/forums/thread.jsp?forum=22&thread=109054&tstart=200&trange=15

0

Andrei ,

This is not the automated solution that you are seeking, but you could
use "Analyze dependencies" to help you do the job manually:
Look for pieces of code that depend on jdbc lib.
=> IDEA will keep a track of all the classes to visit => you won't miss any
=> IDEA will display yellow stripes in the right bar, for each line that
you need to check.

I understand this is a very generic solution. There may be a sharper
knife, for you very specific problem.

Alain

0

An ugly way using structural search:

1. replace {$con$.open(); $code$}
with {$con$.open(); $code$; $con$.close(); }

2. replace $con$.close();$con$.close();
with $con$.close();

-- dimiter

0

Hello Jonas,

Yep, that's exactly what I'm looking for, I can't wait to see it implemented.

Simple Monster, any ETA for it?

Thanks,
Andrei

JK> It seems the Simple Monster had/has some plans for this:
JK> http://intellij.net/forums/thread.jsp?forum=22&thread=109054&tstart=
JK> 200&trange=15
JK>


0


Yeah, well. Although I understand the value of this one (it's easily the highest yield inspection remaining on my TODO list), it's turning out to be trickier than I thought. On the other hand, the new Plugin module/Dev kit stuff is making my plugin coding about five times more efficient, so I have high hopes. Overall, expect this one as a New Year's present, although the first cut is more likely to be two specific inspections for IO streams and JDBC resources than the generic "paired operations" idea.

(For U.S. Thanksgiving, though, everyone can be thankful for MetricsReloaded 0.5, now with class and package cycle detection, analysis, reporting, and visualization. Announcement as soon as I finish the QA cycle.)

--Dave Griffith

0

For those interested, inspections have been added to IG for I/O resources (Streams, Readers, Writers) and JDBC resources (Connections, Statements, ResultSets), which are not opened in a "try" block and closed in the corresponding "finally" block. These inspections should be available in the next EAP. These were fairly tricky to implement, so I'd love to hear about any issues that people have with them.

If anyone has any other ideas for common operation pairs that should be inspected for, I'd love to hear them. There turned out to be enough special issues with these that I currently consider a generic "paired operations must be in try/finally" to be rather unlikely.

--Dave Griffith

0

Hi Dave,

If anyone has any other ideas for common operation
pairs that should be inspected for, I'd love to hear
them.


Do you have inspections for Channels, RandomAccessFiles, Sockets and the new java.util.concurrent Locks too?

Bas

0


Not as of yet. Channels and Sockets were on my TODO list, and RandomAccessFiles are now that you mention them. The java.util.concurrent stuff seems problematic, however. The point making Lock a first class object is so it can be used in those cases where acquire/release operations can't be lexically paired. If you could lexically pair the operations, you're often better off with just a "synchronized" block. Things aren't quite that simple ( locks give you balking, timeout, and some other semantics you can't get out of the "synchronized" keyword), but it's a tougher case than the others. I'll probably let it stew for a while and get responses on the existing inspections before I implement it.

--Dave Griffith

0

Dave Griffith wrote:

For those interested, inspections have been added to IG for I/O
resources (Streams, Readers, Writers) and JDBC resources
(Connections, Statements, ResultSets), which are not opened in a
"try" block and closed in the corresponding "finally" block.


That's great! I've been looking forward to this. Especially in the
context of inspecting other people's code (such as students handing in
their projects where I want to make sure they've listened to what I've
said about releasing resources)...

I'll make sure to take a close look at the inspections as soon as I get
the new EAP version.

0

Is there any plan to allow the user to specify any two arbitrary methods as the paired operations?

I'm asking this because we have some of our own resource management classes which have similar issues to the jdbc connections. Also, even for jdbc, we are using a connection pool class; We need to match the pool.getConnection() with pool.releaseConnection().

I was trying to accomplish the same using Structural Search, but I haven't gotten it to work.

0

Plans, yes, but it won't be in the next EAP.

--Dave Griffith

0

I was trying to create a structural search for something similar--to find all code that calls getConnection() on a pool class and doesn't call freeConnection(), but I haven't been able to get it to work with structural search. I think it might not support the "finally" clause, because I can get a search to work just using try-catch block, but as soon as I add finally clause, it says "This pattern is malformed or unsupported".

0

Alex wrote:

} finally($FinallyExType$ $FinallyExDcl$) {


The "finally" clause doesn't take an exception argument.

0

Thanks..duh!
After fixing the "finally" clause, the search now runs but doesn't find anything.

I started to break it down into components and verifying each part worked.

I noticed that for

with constraint on $getConnection$, text match "getConnections", the search on just this part still turned up nothing. But if I removed the semi-colon at the end, then it worked! Very odd.


After verifying the clauses to match the getConnection() call and the freeConnection() call, I then tried to combine them, first without even considering try-catch-finally blocks.
Here is some example code I am trying to match:
0) status = true; } pool.freeConnection(con); rs.close(); statement.close(); ]]>

Here was my first attempt, by just combining the individual searches which worked plus adding a "$Statement$" which matches any number of statements in between.

When I try to search it says "The pattern is malformed or unsupported.". I then added a semi-colon after the $Statement$ clause

Now the search ran, but returned no results. I also tried with a semi-colon after the getConnection() and freeConnection() lines, but that didn't work either -- and as I mentioned above when I ran each separately with a semi-colon it short-circuited the search somehow.

I guess if I can't even get the above simplified search to work, I shouldn't even try to get the more complicated try-catch-finally to work.

Also, I was looking over our company's 1 million+ line cobebase, and found all kinds of variations of where the getConnection() call is placed in relation to the freeConnection() call. Example:


I'm not sure I could create a structured search that would handle all the possibilities. Ideally, what I would really like is to just do a "Find Usages" on getConnection() method, and then within those search results apply a structured search to match all the usages which follow the pattern exactly (calling getConnection() in try{} block and calling freeConnection() in matching finally{} block), leaving me with the rest to look through manually. IntelliJ would quickly elminate probably 99% of the 1300 usages of getConnection(), allowing to focus on the remaining 1% which may be incorrect. Also, I would feel confident that I didn't miss anything by using the structural search to subtract from all the known usages rather than counting on structural search to show me all the bad usages using a complicated search template.

In our codebase we have 1100 usages of getConnection(). Probably 98% follows the recommended pattern

0

Hi,

1. $getInstance$.$getConnection$($getParameters$); is statement and
$getInstance$.$getConnection$($getParameters$) is expression contained
in some statement
2.
freeParameters variable should be referenced with $freeParameters$ in
last line of following code

$getInstance$.$getConnection$($getParameters$)
$Statement$;
$freeInstance$.$freeConnection$(freeParameters$)


Alex wrote:

Thanks..duh!
After fixing the "finally" clause, the search now runs but doesn't find anything.

I started to break it down into components and verifying each part worked.

I noticed that for
$getInstance$.$getConnection$($getParameters$); > ]]>
with constraint on $getConnection$, text match "getConnections", the search on just this part still turned up nothing. But if I removed the semi-colon at the end, then it worked! Very odd.
$getInstance$.$getConnection$($getParameters$) > ]]>

After verifying the clauses to match the getConnection() call and the freeConnection() call, I then tried to combine them, first without even considering try-catch-finally blocks.
Here is some example code I am trying to match:
Connection con = pool.getConnection(); > Statement statement = con.createStatement(); > ResultSet rs = statement.executeQuery(sql.toString()); > if (rs.next()) { > int count = rs.getInt(1); > if (count > 0) status = true; > } > pool.freeConnection(con); > rs.close(); > statement.close(); > ]]>

Here was my first attempt, by just combining the individual searches which worked plus adding a "$Statement$" which matches any number of statements in between.
$getInstance$.$getConnection$($getParameters$) > $Statement$ > $freeInstance$.$freeConnection$(freeParameters$) > ]]>
When I try to search it says "The pattern is malformed or unsupported.". I then added a semi-colon after the $Statement$ clause
$getInstance$.$getConnection$($getParameters$) > $Statement$; > $freeInstance$.$freeConnection$(freeParameters$) > ]]>
Now the search ran, but returned no results. I also tried with a semi-colon after the getConnection() and freeConnection() lines, but that didn't work either -- and as I mentioned above when I ran each separately with a semi-colon it short-circuited the search somehow.

I guess if I can't even get the above simplified search to work, I shouldn't even try to get the more complicated try-catch-finally to work.

Also, I was looking over our company's 1 million+ line cobebase, and found all kinds of variations of where the getConnection() call is placed in relation to the freeConnection() call. Example:
Connection con = pool.getConnection(); > .. > try { > pool.freeConnection(con); > } > catch() { > } > ]]>

I'm not sure I could create a structured search that would handle all the possibilities. Ideally, what I would really like is to just do a "Find Usages" on getConnection() method, and then within those search results apply a structured search to match all the usages which follow the pattern exactly (calling getConnection() in try{} block and calling freeConnection() in matching finally{} block), leaving me with the rest to look through manually. IntelliJ would quickly elminate probably 99% of the 1300 usages of getConnection(), allowing to focus on the remaining 1% which may be incorrect. Also, I would feel confident that I didn't miss anything by using the structural search to subtract from all the known usages rather than counting on structural search to show me all the bad usages using a complicated search template.

In our codebase we have 1100 usages of getConnection(). Probably 98% follows the recommended pattern



--
Best regards,
Maxim Mossienko
IntelliJ Labs / JetBrains Inc.
http://www.intellij.com
"Develop with pleasure!"

0

+
1. $getInstance$.$getConnection$($getParameters$); is statement and
$getInstance$.$getConnection$($getParameters$) is expression contained
in some statement
+

Shouldn't either match the following statement:

variables:
$getInstance$ constraints: Occurrences 1 to max
$getConnection$ Occurrences 1 to 1, match text "getConnection", whole word
$getParameters$, Occurrences: 0 to 1

It only works without the semi-colon.


+
2.
freeParameters variable should be referenced with $freeParameters$ in
last line of following code

$getInstance$.$getConnection$($getParameters$)
$Statement$;
$freeInstance$.$freeConnection$(freeParameters$)
+

Ooops -- please disregard that -- that was a typo introduced when I edited the name after copying&pasting. It's easy to catch in IntelliJ, because they syntax coloring displays the variables differently.

0

Maxim,
I think I understand what you were saying now.

I was trying to match the following code:

with the template:

(I made getConnection literal since it wasn't necessary to be a variable.)
What you are saying is that if you have the semi-colon (;)
then the entire line of code has to match? Ok, I understand, but that wasn't clear from the Help documentation. You might want to clarify that adding semi-colon (;) forces the entire line to match.

I got the following template to work to match the entire line:

But for my purposes, I would like to use the expression matching because I'm trying to approximate "Find Usages" as closely as possible because I don't want to miss any usages of "getConnection".

That works to match occurrences of getConnection(), but I'm still not getting anywhere trying to match getConnection() with a freeConnection() in the same scope, or (better) match getConnection() without a freeConnection() in the same scope, or (goal) match getConnection() without a freeConnection() in finally clause() in same scope..

The most I have been able to do is to match

with the template matching the two statements exactly:

But that isn't very useful because it only matches when the lines are right after each other.

For the following code:
0) status = true; } pool.freeConnection(con); ]]>
I tried to match with

But it doesn't work. Does "$Statement$" only match a single statement line, or can it match any number of lines?

I guess I am looking for some kind of hybrid between structured search and regex search.

I would like structured search to match the try{} finally{} blocks and verify that $pool$ variable is of type DBConnectionPool, but everything else is regexp matching.

0

Please sign in to leave a comment.