Chris,
See my reply marked DGS below.
Dan
-----Original Message-----
From: Christopher Schultz <ch...@christopherschultz.net>
Sent: Monday, August 11, 2025 3:32 PM
To: users@tomcat.apache.org
Subject: Re: How to access a REST service
Dan,
On 8/7/25 10:29 PM, Daniel Schwartz wrote:
> Robert, Chris, and others who have responded to my queries,
>
> Everyone is convinced that I have problem with my code. Here are what I
> consider to be the relevant code fragments. I've also indicated where I
> consider it to be possible that there might be a memory leak, but I'm not
> sure about this.
>
> The main file is HolidaysRESTJSONResource.java. Following is the beginning
> fragment, which opens a database connection, executes a query to retrieve a
> list of countries, and closes the connection. The constructor for this class
> creates a DataSource by means of a DataSourceSingleton, also copied below.
> This DataSource object is used in HolidaysRESTJSONResource.java to create a
> Connection object, which is then passed to a method GetCountryList.doIt(),
> which creates a prepared statement, this being a DB query to retrieve a list
> of countries, and returns this list. I haven't copied this file here, but I
> did write in some print statements where the prepared statement is created
> and then closed. You will see these in the output log copied below.
>
> I also wrote in some print statements immediately after the Connection object
> is created and immediately after the Connection is closed. I believe that
> this is working as it should, i.e., at this point the Glassfish pooling
> system should mark this Connection object as no longer in use and therefore
> available for future use. If there is a memory leak, then this is the most
> likely place. In any case, I do execute a close operation on this Connection
> object and print out a statement about this. You can also see this in the
> output log.
>
> For completeness I also copied the TempDataStorage.java, in case you're
> curious about this. It simply holds the countryList so that the system
> doesn't need to keep retrieving it.
>
> The HolidaysRESTJSONResource.java has five method calls like the one shown
> here as GetCountryList.doIt(). All of them have the same code for opening
> and closing a connection. I also put some print statements in the method
> that returns the holidays list. These also appear in the log file.
>
> Upon reflection, I think that the only place where a memory leak might occur
> is in the place I indicated above---that closing the Connection object in the
> way I have done might not actually have that object be marked as no longer in
> use in the pool. However, not knowing enough about these pooling systems, I
> really don't know.
>
> What do you think?
You definitely have connection leaks in your code. For example:
> public String getJsonCountries() {
> if (TempDataStorage.countryList == null) {
> Connection connection = null;
> try {
> connection = dataSource.getConnection();
Exception occurring after here will leak a connection.
> System.out.println("countries connection has been opened");
> TempDataStorage.countryList =
> GetCountryList.doIt(connection);
> connection.close();
> System.out.println("countries connection has been closed");
> } catch (SQLException e) {
> System.out.println(e);
> if (connection != null) {
> try {
> connection.close();
> } catch (SQLException ex) {
> System.out.println(ex);
> }
> }
> }
> }
You should have a connection.close() in a finally{} block which is missing in
this code. You also need to catch other types of exceptions:
- RuntimeException
- Error
- Anything else that your method is declared to throw
(currently nothing)
DGS: You point is well taken. I will add a finally block. However, as far as I
know, no exceptions are being thrown. If a Java program throws an exception
that isn't being caught, the program crashes and outputs a stack trace. This
has never happened.
> StringListWrapper listWrapper = new StringListWrapper();
> listWrapper.setTheList(TempDataStorage.countryList);
> return new Gson().toJson(listWrapper);
> }
>
> ...
> ----------------------------------------------------------------------
> ---------
> DataSourceSingleton.java
> ...
> public class DataSourceSingleton {
>
> private static DataSourceSingleton instance = null;
> public DataSource dataSource = null;
>
> private DataSourceSingleton() {
> try {
> InitialContext ctx = new InitialContext();
> dataSource = (DataSource) ctx.lookup("jdbc/holidays");
> } catch (NamingException e) {
> System.out.println(e);
> }
> }
>
> public static DataSourceSingleton getInstance() {
> if (instance == null) {
> instance = new DataSourceSingleton();
> }
> return instance;
> }
> }
You probably need some "synchronized" keywords in there. If this were my code,
I would do:
private static final DataSourceSingleton instance = new DataSourceSingleton();
...and then remove the predicate in getInstance, just returning that
already-initialized static class member.
> ----------------------------------------------------------------------
> ----------
> server.log
>
> [2025-08-07T21:19:43.526-0400] [glassfish 4.1] [INFO] [] [] [tid:
> _ThreadID=39 _ThreadName=Thread-8] [timeMillis: 1754615983526] [levelValue:
> 800] [[
> countries connection has been opened]]
>
> [2025-08-07T21:19:43.528-0400] [glassfish 4.1] [INFO] [] [] [tid:
> _ThreadID=39 _ThreadName=Thread-8] [timeMillis: 1754615983528] [levelValue:
> 800] [[
> creating getCountries prepared statement]]
>
> [2025-08-07T21:19:43.529-0400] [glassfish 4.1] [INFO] [] [] [tid:
> _ThreadID=39 _ThreadName=Thread-8] [timeMillis: 1754615983529] [levelValue:
> 800] [[
> closing getCountries prepared statement]]
>
> [2025-08-07T21:19:43.529-0400] [glassfish 4.1] [INFO] [] [] [tid:
> _ThreadID=39 _ThreadName=Thread-8] [timeMillis: 1754615983529] [levelValue:
> 800] [[
> countries connection has been closed]]
>
> [2025-08-07T21:20:19.635-0400] [glassfish 4.1] [INFO] [] [] [tid:
> _ThreadID=36 _ThreadName=Thread-8] [timeMillis: 1754616019635] [levelValue:
> 800] [[
> holidays connection has been opened]]
>
> [2025-08-07T21:20:19.704-0400] [glassfish 4.1] [INFO] [] [] [tid:
> _ThreadID=36 _ThreadName=Thread-8] [timeMillis: 1754616019704] [levelValue:
> 800] [[
> holidays connection has been closed]]
Are you seeing any opens without closes?
When it "crashes", what do you get? I'm assuming an exception stack trace.
-chris
> -----Original Message-----
> From: Robert Turner <rtur...@e-djuster.ca.INVALID>
> Sent: Thursday, August 7, 2025 5:07 PM
> To: Tomcat Users List <users@tomcat.apache.org>
> Subject: Re: How to access a REST service
>
> Dan,
>
> On Thu, Aug 7, 2025 at 5:01 PM Daniel Schwartz
> <d...@danielgschwartz.com>
> wrote:
>
>> Hello Chris,
>>
>> Thank you for your reply, but I'm still unsure. You seem to be
>> implying that I have a memory leak, i.e., many connection objects
>> being created that are not being closed. However, I really don't
>> think this is happening. My code closes each connection immediately after
>> using it.
>
>
> Maybe post your code again. Last time you posted it, it was prone to leaking
> connections. If it hasn't changed, that is likely your problem.
>
>
>>
>> My understanding is that the only way the maximum pool size of X,
>> whatever that is, would be a limitation is if there was an attempt to
>> create X+1 simultaneous connections. When you do this in Glassfish,
>> it outputs an error message saying that no more connections can be
>> created and then crashes. You have to go back in and manually restart it.
>>
>> I believe that the essential problem, as explained in a previous
>> email to Rob Sargent, is that I'm getting several hundred database
>> requests per day from web crawlers. I just spent some time reading
>> through my ngnix access.log and found that the vast majority of these are
>> from GoogleBot.
>> My guess it that, due to a time lag between opening and closing
>> connections, many connections will be opened simultaneously. This is
>> why a small pool size won't work.
>>
>> Also, I'm advised to not block the web crawlers because this assists
>> with SEO. My understanding is that you just have to live with this.
>>
>> I don't think there is an issue with my code. The only answer I can
>> come up with is to have a large maximum pool size, larger that the
>> expected number of simultaneous accesses.
>>
> There is almost definitely a problem with your code (unfortunately), or your
> database requests are very slow and triggered by any connection.
>
> We run servers that handle much more traffic than you are describing and make
> thousands of DB requests per minute, and we rarely go over 10 DB connections
> being used at a time.
>
> There is almost for sure something leaking in your code. This is very
> unlikely to be a problem with the pooling ("select isn't broken"). You are
> looking for unlikely causes to the problem.
>
>
>>
>> I originally wrote to this email list because I was thinking of
>> shifting from Glassfish to Tomcat, and was trying to learn how to do
>> this. I think I do know how to do this now, and might try doing this.
>> My understanding is that the connection pooling that works with
>> Tomcat doesn't have that same limitation as Glassfish, and one can
>> have connections that exist outside the pool. This would resolve the
>> issue I'm currently having with Glassfish.
>>
>> Best regards,
>>
>> Dan Schwartz
>> ...snip..
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: users-h...@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
