Commit Session on Include Dispatch calls commitSession each time

[laurentschoelens]

Describe the bug
First, I think it's not clearly a bug but a performance issue that may be optimized.

After the fix of #1242, commitSession has been called each time a RequestDispatcher.include is called. When using jsp, jsp:include do calls RequestDispatcher.include and then if you have massive usage of spring session getting and saving session to backend.

To Reproduce
Having a project with jsp and jsp:include tags.
Each jsp:include tag calls commitSession.

Expected behavior
Since the initial issue has to still be fixed, I think, once it has saved the session, it should keep track of this and no more save the session in RequestDispatcher.include until the request has finished processing (which is done in the filter in the finally block).

Sample
No simple sample to provide, sorry.
I'll try submitting a PR for this.

[laurentschoelens]

Hi, please let me know if something is missing in the issue or in the pull request
I'll try to create a simple example to show the problem I'm facing

[marcusdacoregio]

Hi @laurentschoelens, I'll try to set up a sample to simulate this. But I'm curious about your application, do you have too many jsp:include? Can you share a bit more of your context?

[laurentschoelens]

Hi @marcusdacoregio
Thanks for your reply.
I've many jsp:include in my app, for sure, and I can't change it (for the moment - planned but not scheduled)
Each main page includes blocks of the page in other jsp files via jsp:include directive, and some of these blocks are calling other jsp:include.

Can't share my app code but I found someone also had the same problem but the sample disappeared from github :(
Still, I've included my moded class in my app with the MR I've provided and the app is running like on fire for now with no bugs so far (1 month by now) so I think it's safe (for me :))

[laurentschoelens]

Hi @marcusdacoregio : any news on this issue ?
Even if I patched myself the class in the application, I'd prefer having a fixed official spring-session jar for stability 😄
Please let me know if I can do something more on this.

PR already provided : #2285

Thanks

[laurentschoelens]

Any news ? Still waiting for a status of my issue and if it'll be fixed ?
Thanks again. Please keep me updated
Regards

[marcusdacoregio]

Hi @laurentschoelens, I'll check the priorities and see if I can add this to the 3.2 release. I still have to go through the details and simulate it myself the check the solution. I appreciate your patience and once it is prioritized you are gonna see the issue assigned to a milestone.

[laurentschoelens]

Hi @laurentschoelens, I'll check the priorities and see if I can add this to the 3.2 release. I still have to go through the details and simulate it myself the check the solution. I appreciate your patience and once it is prioritized you are gonna see the issue assigned to a milestone.

Thanks for your reply. I'll try to provide with some simple example for you to see the issue.
Any chance this go to 2.7.x maintenance branch. It's a big issue (for me) and migration to springboot 3 (with jakarta and all the dependencies) is just too complex for the moment.
Regards. Laurent

[laurentschoelens]

Here's a simple POC that reveals the massive updates that are done in the application after each jsp:include.

gh-2284.zip

Just navigate to http://localhost:8080/index after starting the app and click on the link Go to include test page that will run a for loop of 10 times jsp:include.
The massives updates will be shown in console log, resulting of more than 10 times session update (4 session update query and 10 session_attributes update query) with Spring-Session JDBC H2 backend.

[laurentschoelens]

@marcusdacoregio : any news on this ?
Put the fixed classes into production and no problem seen : only better response time and less memory footprint :)

[marcusdacoregio]

Hi @laurentschoelens, not yet. As I mentioned, when we have the rationale to decide if it is an enhancement or a bug you are going to see the milestone updated.

[laurentschoelens]

@marcusdacoregio : PR updated to what wanted 😄

[marcusdacoregio]

Closing in favor of #2285

[laurentschoelens]

@marcusdacoregio : thanks for the fix, do we have a chance to have a backport on 2.7 springboot ?