B4J Question Does this look like a memory leak?

[j_o_h_n]

I have a B4J server app using Jetty on a Windows VPS
The database it uses is Postgres.
In the pgadmin tool you can see the connections and the current query for each of those.
What I am noticing is that some older connections never show a current query.

I've gone through the code several times and all sql connections and resultsets
are locally declared and all are closed before exiting the sub

Dim sql1 As SQL = pool.GetConnection

    Dim RS As ResultSet

   

    Try

    Catch

    End Try

    RS.Close

    sql1.close

    Return s

Last week I started monitoring the app using JConsole. Below are 3 screenshots
from it, the overview, the heap memory usage and the non-heap memory usage.
You can see that a few days ago the memory usage suddenly increased and has
been more volatile since then.
The request logs show a steady number of requests. although last weekend (the first few days )
were quieter because of the Easter Holidays.
I expect memory usage to jump again and at some point I'll have to restart the app because it
will have become unresponsive.

 

[Erel]

I expect memory usage to jump again and at some point I'll have to restart the app because it
will have become unresponsive.

This means that there is a leak somewhere.
Server apps can run forever without needing to be restarted.

What exactly happens when the server becomes unresponsive? Do you see any error message in the logs?

 

[j_o_h_n]

This means that there is a leak somewhere.
Server apps can run forever without needing to be restarted.

What exactly happens when the server becomes unresponsive? Do you see any error message in the logs?

Thank you Erel.
I don't think I have that information currently.
I have just been concerned with restarting and not saving stuff.
I have some request logs and also some logging in the command window from the current run.
The only logged exception I seem to see is a recurrent "(List) Not initialized" one.

 

[j_o_h_n]

Looking for possible memory leak causes I was wondering about having the declaration and initialization of SQL variables before the Try statement.
An exception on that line (such as if the database server wasn't responding) would not be caught in the subsequent Catch section I thought.
To test it I wrote a couple of small subs, one calls the other
I stopped the database server just before the SQL variable declaration line was executed.
I expected the exception to trigger the Catch clause in the Outer sub and thus bypass the SQL.Close statement but it was actually 'caught' in the Catch section
of the Try-Catch construct that it was outside of?

I guess that is the way that it is supposed to be, it just struck me as odd.

Sub Outer_Sub
    Try
        Inner_Sub '<--- I stopped the database server here in order to cause an exception in Inner_Sub
    Catch
        Log(LastException)   '<---the second exception is logged here, I expected the first one to be logged here and no second exception.
    End Try
  
End Sub

Sub Inner_Sub
 
    Dim sql1 As SQL = pool.GetConnection  '<-------execution hangs on this line because database server is stopped
    Try
     
    Catch
     
        Log(LastException)  '<----until finally this exception is logged here: An attempt by a client to checkout a Connection has timed out.
                                          ' I wasn't expecting this catch section to be executed at all.

    End Try
 
    sql1.close  ' <----- this line then has a NullPointerException which is logged in the catch section of the outer function
 
End Sub

 

[j_o_h_n]

Assuming that not closing a SQL object causes a memory leak then I think this pattern that I use a lot might be one cause

Dim sql1 As SQL = pool.GetConnection
Dim RS As ResultSet

    Try
'if there's an exception in here before the resultset is initialized
    Catch
    End Try
    RS.Close 'then another exception occurs here
    sql1.close 'and this line is not executed

So one change I'm going to make to is replace all instances of

RS.close

with

If RS.IsInitialized Then RS.close

 

[j_o_h_n]

I suspect that the above may well be my problem.
The connection pool size is limited to 15 connections
I ran the above code in a loop and on the 16th and subsequent calls, exception messages were all slow coming and all were about a connection timeout.
The picture underneath from pgadmin showing the connections, shows 15 with no current query which is the sort of pattern I see on the real server.
I suspect the server app copes while there's even a single connection available and then it stops responding altogether once that goes, as every http request requires a database lookup

15 java.lang.RuntimeException: Object should first be initialized (ResultSet).

16 com.mchange.v2.resourcepool.TimeoutException: A client timed out while waiting to acquire a resource from com.mchange.v2.resourcepool.BasicResourcePool@179ece50 -- timeout at awaitAvailable()

 

[Erel]

So one change I'm going to make to is replace all instances of

True. Better to close the ResultSet inside the the Try block. The SQL.Close method is the important one.

 

[j_o_h_n]

True. Better to close the ResultSet inside the the Try block. The SQL.Close method is the important one.

Good idea, thank you, that's what I'll do

 

[Magma]

@j_o_h_n ...hmmm... also always "limit" the results of rows - if sqlite use "SELECT * FROM TABLE LIMIT x;"

x can be one (1) - if you want only one result... or specify X with the number of rows you wanna receive... is very important too...

* others dbs may differ... not using LIMIT but TOP...

 

[j_o_h_n]

@j_o_h_n ...hmmm... also always "limit" the results of rows - if sqlite use "SELECT * FROM TABLE LIMIT x;"

x can be one (1) - if you want only one result... or specify X with the number of rows you wanna receive... is very important too...

* others dbs may differ... not using LIMIT but TOP...

Hi Magma I see your point but mostly I rely on a WHERE clause to put a limit on the number of rows returned. I'd use limit
if I wanted, say, the last row in a table, something like "select * from tablename order by ID Desc Limit 1"

 

[Magma]

Hi Magma I see your point but mostly I rely on a WHERE clause to put a limit on the number of rows returned. I'd use limit
if I wanted, say, the last row in a table, something like "select * from tablename order by ID Desc Limit 1"

ofcourse you can use WHERE and LIMIT too...

and ofcourse sometimes need to show all the rows...

and sometimes LIMIT to 10 or 100....

* I ve just telling about that... because... is a problem for "memory leaks" too..

 

[j_o_h_n]

* I ve just telling about that... because... is a problem for "memory leaks" too..

Hi Magma
Can you clarify this for me please?
1: Are you saying that if I have a resultset and it isn't closed properly, that a memory leak will occur?
2: And then obviously, the larger the resultset, the larger the leak, which is why using LIMIT helps?

If that is true then I think that putting the closing of the resultset inside the TRY block, as Erel suggested, would be a mistake
as the close method would not be called, if an exception occurred earlier in the Try block.
That I would need to ensure that BOTH the closing of the resultset and the closing of the SQL Object occurred?
That something like the code below, with nested TRY CATCH constructs would be better?
In the code below RS is the resultset and SQL is the SQL object.

Try
    Try
        'The real work happens here
    Catch
    End Try
    RS.Close
Catch
End Try
SQL.Close

 

[Magma]

Well I am not saying (I really can't - because Erel is always 10 steps forward, at least)...

Also i don't know the full code and how it works - for example if the code is executed when a client and others (the same time) asks something from server... that means that may be the "query" executing the same time with some other...

For me the best way working - is "RS QUERY (limited)" - "RS CLOSE" and then ask for new | per client - ofcourse sometime queries must execute parallel...
But for sure when get query results - must close that.... to be sure... and not forget it...

Try Catch... needed because you never know if server is availiable.. sometimes there are problems can't predict...

 

[Magma]

Also... I think that using sql (opening db) one time, right ?
* and not everytime when client asks (queries)...

That seems ok for me...

    Try
        'The real work happens here

    Catch

    End Try
    RS.Close

or that:

    Try
        'The real work happens here
    RS.Close
    Catch
...
    RS.Close
    End Try

 

[j_o_h_n]

Hi Magma
To be honest I'm a little fuzzy on memory leaks.
From Erel I understand that the most important thing is to ensure that the SQL connection is closed.
From you I understand that it is important to close resultset objects.
If both are true then nested TRY CATCH END TRY blocks, ensuring that both methods are called, appears to be the way to go.
Correct?