[
https://issues.apache.org/jira/browse/WW-5312?focusedWorklogId=862511&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-862511
]
ASF GitHub Bot logged work on WW-5312:
--------------------------------------
Author: ASF GitHub Bot
Created on: 27/May/23 19:33
Start Date: 27/May/23 19:33
Worklog Time Spent: 10m
Work Description: JCgH4164838Gh792C124B5 commented on code in PR #688:
URL: https://github.com/apache/struts/pull/688#discussion_r1208111764
##########
core/src/main/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptor.java:
##########
@@ -394,7 +404,10 @@ public void init() {
@Override
public void destroy() {
- super.destroy();
- executor.shutdown();
+ try {
+ executor.shutdown();
+ } finally {
+ super.destroy();
+ }
Review Comment:
Hi Lukasz. Thanks for reviewing the proposed fix changes. 😄
Looking at the commit history, `SessionMap` has extended `AbstractMap`
since its earliest commits (many, many years ago). Since `SessionMap` is
_public_ and has been structured a certain way for so long, changing its design
to delegate (to a non-accessible internal map, instead of extending) could have
unexpected impacts (especially for anything that might try to use/extend it in
user-space code).
Staying with the long-standing (preexisting) design might be safer in terms
of potential side-effects. Keeping `@overrides` annotations in place along
with some unit tests that attempt to detect breaking-changes should help.
Considering a `SessionMap` redesign would probably need to be a standalone
issue/work item, if people think it is needed (taking into account the existing
design seems to have worked sufficiently for a long time).
Hopefully my reasoning above makes sense. If it seems like other
changes/improvements should be made to the PR, please let me know.
Issue Time Tracking
-------------------
Worklog Id: (was: 862511)
Time Spent: 40m (was: 0.5h)
> ExecuteAndWaitInterceptor inconsistent wait processing behaviour
> ----------------------------------------------------------------
>
> Key: WW-5312
> URL: https://issues.apache.org/jira/browse/WW-5312
> Project: Struts 2
> Issue Type: Bug
> Components: Core, Core Interceptors
> Affects Versions: 6.1.2
> Environment: Java 8, Tomcat 8.5. The behaviour will likely be the
> same for other Java/Server combinations.
> Reporter: James Chaplin
> Priority: Minor
> Labels: pull-request-available
> Fix For: 6.2.0
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> The _ExecuteAndWaitInterceptor_ processing appears to only execute as
> expected for the first run-through for any given session.Â
> This can be seen interactively using the Struts2 Showcase "_Execute and Wait
> Examples_", with the wait processing only functioning during the first
> attempt through each example in the same session. Clearing the session in
> the browser gives the expected wait behaviour again, but only one time (until
> clearing the session again).
> After debugging the _ExecuteAndWaitInterceptor_ flow, it appears that the
> removal from the _SessionMap_ did not perform as expected, resulting in a
> state divergence with the underlying session.
> A PR that attempts to fix the behaviour will be submitted soon.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
