You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2019/04/08 14:37:27 UTC
[Bug 63324] New: CrawlerSessionManagerValve is getting put into
Session, which causes problems when serializing sessions
https://bz.apache.org/bugzilla/show_bug.cgi?id=63324
Bug ID: 63324
Summary: CrawlerSessionManagerValve is getting put into
Session, which causes problems when serializing
sessions
Product: Tomcat 8
Version: 8.5.x-trunk
Hardware: PC
OS: Linux
Status: NEW
Severity: minor
Priority: P2
Component: Catalina
Assignee: dev@tomcat.apache.org
Reporter: martin.lemanski@willhaben.at
Target Milestone: ----
Created attachment 36512
--> https://bz.apache.org/bugzilla/attachment.cgi?id=36512&action=edit
the stacktrace, which indicates the serialization
We are using a serialization library to serialize our sessions between multiple
tomcat instances.
During our OpenJDK 11 upgrade, we've found out, that the
CrawlerSessionManagerValve is in the session, which has dependencies to JDK
internals.
We don't want to serialize JDK internals between server ups and downs.
I've already created a PR to fix this issue, by providing a smaller class to
the session attributes.
https://github.com/apache/tomcat/pull/154
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 63324] CrawlerSessionManagerValve is getting put into Session,
which causes problems when serializing sessions
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63324
--- Comment #2 from Konstantin Kolinko <kn...@gmail.com> ---
Created attachment 36515
--> https://bz.apache.org/bugzilla/attachment.cgi?id=36515&action=edit
154.patch (a copy of PR 154)
A copy of the current code in PR 154, formatted as a patch.
I'll comment on it, so let's keep a copy here.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 63324] CrawlerSessionManagerValve is getting put into Session,
which causes problems when serializing sessions
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63324
--- Comment #1 from Martin L <ma...@willhaben.at> ---
For a more broad understanding:
We have a mem-cached session serialization strategy for our multiple tomcat
instances. When one server goes down, we serialize the sessions into memcache
and put it on a different server which is still running.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 63324] CrawlerSessionManagerValve is getting put into Session,
which causes problems when serializing sessions
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63324
--- Comment #3 from Konstantin Kolinko <kn...@gmail.com> ---
Note that the assumption of CrawlerSessionManagerValve is that the clients does
not support cookies. Thus it forcefully assigns them to the same session based
on their IP addresses.
[[[
if (isBot) {
sessionId = clientIdSessionId.get(clientIdentifier);
]]]
1) If the client really does not support cookies, once you stop and start
Tomcat, the "clientIdSessionId" map is lost and you have lost access to those
sessions. They will never be accessed again.
They will just time out after some time elapses - "sessionInactiveInterval" in
CrawlerSessionManagerValve defaults to 60 seconds.
(Thus it makes sense to do not serialize those sessions at all, to do not
replicate them etc.
Serializing the original maps (like proposed by PR 154) does not make any
sense.)
2) If client supports cookies, you do not need a CrawlerSessionManagerValve.
Thus you not not need a value in a "clientIdSessionId" map. You will access the
session using the sessionid provided by a Cookie.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 63324] CrawlerSessionManagerValve is getting put into Session,
which causes problems when serializing sessions
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63324
--- Comment #8 from Martin L <ma...@willhaben.at> ---
Created attachment 36523
--> https://bz.apache.org/bugzilla/attachment.cgi?id=36523&action=edit
patch for 7.0.x (PR 156)
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 63324] CrawlerSessionManagerValve is getting put into Session,
which causes problems when serializing sessions
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63324
--- Comment #7 from Martin L <ma...@willhaben.at> ---
Created attachment 36522
--> https://bz.apache.org/bugzilla/attachment.cgi?id=36522&action=edit
patch for 8.5.x
adding patch for 8.5.x (PR 155)
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 63324] CrawlerSessionManagerValve is getting put into Session,
which causes problems when serializing sessions
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63324
--- Comment #4 from Konstantin Kolinko <kn...@gmail.com> ---
Based on the analysis in comment #3 I think that a possible solution is to
adjust the original proposal as follows. I am quoting fragments from attachment
36515
[[[
+ private static class CrawlerHttpSessionBindingListener implements
HttpSessionBindingListener {
+ private final Map<String, String> clientIdSessionId;
+ private final Map<String, String> sessionIdClientId;
]]]
Three changes are needed:
1. Declare CrawlerHttpSessionBindingListener to implement java.io.Serializable.
2. The "clientIdSessionId" field should be declared transient.
3. The "sessionIdClientId" field does not need to be a Map. Just add a String
field "String clientIdentifier" that stores a single value. The field can be
transient as well.
[[[
+ @Override
+ public void valueUnbound(HttpSessionBindingEvent event) {
+ String clientIdentifier =
sessionIdClientId.remove(event.getSession().getId());
+ if (clientIdentifier != null) {
+ clientIdSessionId.remove(clientIdentifier);
+ }
}
]]]
4. Add a check that clientIdSessionId is not null.
Duplicate removals can he handled by using method Map.remove(key, value) that
removes a key only if the value matches.
if (clientIdentifier != null && clientIdSessionId != null) {
clientIdSessionId.remove(clientIdentifier, event.getSession().getId());
}
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 63324] CrawlerSessionManagerValve is getting put into Session,
which causes problems when serializing sessions
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63324
Martin L <ma...@willhaben.at> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |martin.lemanski@willhaben.a
| |t
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 63324] CrawlerSessionManagerValve is getting put into Session,
which causes problems when serializing sessions
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63324
--- Comment #5 from Martin L <ma...@willhaben.at> ---
Thanks for your feedback.
I've applied your suggestions, but needed to customize them a bit, depending on
the branch.
There are 3 PRs, one for 7.0.x, one for 8.5.x and one for master.
In 7.0.x the test setup looks a slightly bit different, and you need to
implement both interface methods of HttpSessionBindingListener
https://github.com/apache/tomcat/pull/156
In 8.5.x I couldn't use the signature Map.remove(key, value).
I was getting a `method remove in interface Map<K,V> cannot be applied to given
types; clientIdSessionId.remove(clientIdentifier, event.getSession().getId());`
when running the tests.
So I needed to check for the same value in an extra if.
https://github.com/apache/tomcat/pull/155
In master is the "cleanest" solution.
https://github.com/apache/tomcat/pull/154
Do you want me to upload all 3 patches here?
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 63324] CrawlerSessionManagerValve is getting put into Session,
which causes problems when serializing sessions
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63324
Martin L <ma...@willhaben.at> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #36515|0 |1
is obsolete| |
--- Comment #6 from Martin L <ma...@willhaben.at> ---
Created attachment 36521
--> https://bz.apache.org/bugzilla/attachment.cgi?id=36521&action=edit
patch for master
adding new patch for PR 154
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 63324] CrawlerSessionManagerValve is getting put into Session,
which causes problems when serializing sessions
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63324
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
--- Comment #9 from Mark Thomas <ma...@apache.org> ---
Thanks for the report and the patches to fix this issue. Updating the test
cases is particularly appreciated.
Fixed in:
- master for 9.0.20 onwards
- 8.5.x for 8.5.41 onwards
- 7.0.x for 7.0.95 onwards
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org