Add try-with-resources to methods to insert BLOB

[k-tamura]

Problem

If using Oracle DB as a session store of Spring Session and idle connection timeout is not set (or connections do not become idle for a long time), there is a possibility that Oracle's TEMP space is increased and finally SQLException (ORA-01652) is thrown.

Note: This problem has already reported on #1321.

Description

When inserting a record to SPRING_SESSION_ATTRIBUTES table (which has Blob column), the following steps are executed:

1. Create a temporary LOB in Oracle's TEMP space
2. Insert a record to SPRING_SESSION_ATTRIBUTES table

The created temporary LOB is freed when the connection (which created the temporary LOB) is closed by idle timeout or Spring application is restarted, but these events do not always occur.

Solution

So the following step should be added just after step 2.

3. Free the temporary LOB

To achieve this, try-with-resources statement should be added.

This problem is caused by leaks of calling TemporaryLobCreator.close(). TemporaryLobCreator implements LobCreator (which extends Closeable) and has close() (which frees temporary LOB), but TemporaryLobCreator is created without try-with-resources statement. Therefor close() is not called and temporary LOB is not freed.

Steps to reproduce:

1. Create a Web application which uses Oracle DB as a session store of Spring Session.
2. Access to the application (for example, 1000 times by JMeter).
3. Run the SQL:

   select * from V$TEMPORARY_LOBS;
   

If you keep accessing to the application, ORA-01652 occurs and SQLException is thrown.

Result before applying my commit (CACHE_LOBS will increase):

   SID CACHE_LOBS NOCACHE_LOBS ABSTRACT_LOBS     CON_ID
---------- ---------- ------------ ------------- ----------
     18	  371		 0	       0	  0
     23	    0		 0	       0	  0
     25	  229		 0	       0	  0
    173	  144		 0	       0	  0
    178	  723		 0	       0	  0
    346	  533		 0	       0	  0

Result after applying my commit (CACHE_LOBS will not increase):

    SID CACHE_LOBS NOCACHE_LOBS ABSTRACT_LOBS     CON_ID
---------- ---------- ------------ ------------- ----------
     23	    0		 0	       0	  0

[rwinch]

Thanks for the PR! Can you please add some tests for the changes?

[k-tamura]

Hi @rwinch ,

Thank you for your reviewing.
My changes will only affect Oracle's TEMP space. So I think it is difficult to create test cases to confirm it. Do you have a good idea?

[rwinch]

I'm guessing you could add a unit test to ensure that the temporary LOG is freed?

[k-tamura]

Some test methods in JdbcIndexedSessionRepositoryTests calls two methods which is changed by me. However, it seems that variable this.jdbcOperations does not include information which indicates the temporary LOG is freed even if createTemporaryLob flag is true. So I think a unit test to ensure it cannot be added.

[rwinch]

I think you would need to verify that the lob that was created (which would be a mock also) is then closed proeprly.

[k-tamura]

I am sure that freeing temporary LOB cannot be tested. If not using Oracle DB, we need to test if TemporaryLobCreator.close() is called while calling this.repository.save(session); but, as far as I investigated, it is not possible on this testing framework.

[k-tamura]

@rwinch

Do you believe it is possible? If yes, could you give me some more specific advice?

[rwinch]

I think it should be possible by injecting a mock LobHandler that returns a mock LobCreator. Then you would verify the LobCreator is closed.

[k-tamura]

@rwinch

Thank you for your advice. I added a test method. Is this what you intended?

[pivotal-issuemaster]

@k-tamura Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-issuemaster]

@k-tamura Thank you for signing the Contributor License Agreement!

[k-tamura]

@rwinch

It would be very helpful if you could comment my test case. I added it on 22 Jul.

Best regards,

[yoshimi0227]

How's the review going?
I'd really appreciate it if this request could be merged !

[k-tamura]

@rwinch @vpavic

Is it difficult to include this commit in the next release?

[rwinch]

We are already in the release process. Given this is a bug, we can get it in the next patch release (Dec 9th). That will be in time for Spring Boot's GA release.

[eleftherias]

Thanks for the PR @k-tamura! This is merged into master.

[k-tamura]

Thank you!