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 2023/10/30 16:59:12 UTC
[Bug 68026] New: org.apache.tomcat.buf.MessageBytes.toString() is no longuer cached
https://bz.apache.org/bugzilla/show_bug.cgi?id=68026
Bug ID: 68026
Summary: org.apache.tomcat.buf.MessageBytes.toString() is no
longuer cached
Product: Tomcat 10
Version: 10.1.15
Hardware: PC
OS: Linux
Status: NEW
Severity: normal
Priority: P2
Component: Servlet
Assignee: dev@tomcat.apache.org
Reporter: lucas.pouzac@lpoconseil.fr
Target Milestone: ------
Created attachment 39323
--> https://bz.apache.org/bugzilla/attachment.cgi?id=39323&action=edit
Java Flight Recording with spring boot application
Since fixing bug 66196, the org.apache.tomcat.buf.MessageBytes.toString() is no
longuer cached.
For one tomcat request, if a multiple calls (by example to this method,
org.apache.catalina.connector.RequestFacade.getMethod() by springframework) is
processed, a memory overconsumption is observed
Maybe just fix with this ?
```
public String toString() {
switch (type) {
case T_NULL:
case T_STR:
// No conversion required
break;
case T_BYTES:
// strValue = byteC.toString(); #OLD
setString(byteC.toString());
break;
case T_CHARS:
// strValue = charC.toString(); #OLD
setString(charC.toString());
break;
}
return strValue;
}
```
Same problem with tomcat
- >= 10.1.0
- >= 9.0.71
- >= 8.5.85
--
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 68026] org.apache.tomcat.buf.MessageBytes.toString() is no longuer cached
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=68026
lucas.pouzac@lpoconseil.fr changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |lucas.pouzac@lpoconseil.fr
--
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 68026] org.apache.tomcat.buf.MessageBytes.toString() is no longuer cached
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=68026
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
--- Comment #5 from Mark Thomas <ma...@apache.org> ---
Fixed in:
- 11.0.x for 11.0.0-M14 onwards
- 10.1.x for 10.1.16 onwards
- 9.0.x for 9.0.83 onwards
- 8.5.x for 8.5.96 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
[Bug 68026] org.apache.tomcat.buf.MessageBytes.toString() is no longuer cached
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=68026
--- Comment #2 from lucas.pouzac@lpoconseil.fr ---
Maybe, it's possible to cache in the caller ?
https://github.com/apache/tomcat/blob/main/java/org/apache/catalina/connector/Request.java
For all coyoteRequest.*.toString() calls ?
--
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 68026] org.apache.tomcat.buf.MessageBytes.toString() is no longuer cached
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=68026
lucas.pouzac@lpoconseil.fr changed:
What |Removed |Added
----------------------------------------------------------------------------
Component|Servlet |Catalina
--
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 68026] org.apache.tomcat.buf.MessageBytes.toString() is no longuer cached
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=68026
lucas.pouzac@lpoconseil.fr changed:
What |Removed |Added
----------------------------------------------------------------------------
Severity|normal |regression
Priority|P2 |P1
--
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 68026] org.apache.tomcat.buf.MessageBytes.toString() is no longuer cached
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=68026
--- Comment #6 from lucas.pouzac@lpoconseil.fr ---
Thanks for your quick fix.
But if you call toStringType() and after toString() method, your toString()
method has not the same result because type attribute has been updated. Maybe
it's necessary to take a specific cache in toStringType() without call
setString() method ?
On the other hand, this should fix the memory consumption problem.
--
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 68026] org.apache.tomcat.buf.MessageBytes.toString() is no longuer cached
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=68026
--- Comment #4 from Mark Thomas <ma...@apache.org> ---
Changes to the behaviour of MessageBytes has previously introduced subtle bugs.
I am therefore extremely reluctant to make the changes suggested in comment #0
or comment #3.
Caching the values as suggested in comment #2 creates the risk that changes
made to the underlying MessageBytes instance will not be seen. I don't think
that is the case with request method but I'd prefer a more general solution.
Rémy's suggestion in comment #1 should work. I'll implement that for request
method. I'll look at the other MessageByte usages in coyote request and may
update some of the other usages as well.
--
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 68026] org.apache.tomcat.buf.MessageBytes.toString() is no longuer cached
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=68026
--- Comment #3 from lucas.pouzac@lpoconseil.fr ---
Or maybe just update with this code to keep the behaviour described in the
getType() Javadoc ?
public void setBytes(byte[] b, int off, int len) {
byteC.setBytes(b, off, len);
type = T_BYTES;
hasHashCode = false;
hasLongValue = false;
strValue = byteC.toString();
}
/**
* Sets the content to be a char[]
*
* @param c the chars
* @param off the start offset of the chars
* @param len the length of the chars
*/
public void setChars(char[] c, int off, int len) {
charC.setChars(c, off, len);
type = T_CHARS;
hasHashCode = false;
hasLongValue = false;
strValue = charC.toString();
}
// -------------------- Conversion and getters --------------------
/**
* Compute the string value.
*
* @return the string
*/
@Override
public String toString() {
return strValue;
}
--
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 68026] org.apache.tomcat.buf.MessageBytes.toString() is no longuer cached
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=68026
Remy Maucherat <re...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Severity|regression |enhancement
--- Comment #1 from Remy Maucherat <re...@apache.org> ---
Actually, the change that actually modified the behavior is the fix for bug
66627 (
https://github.com/apache/tomcat/commit/897931b68c89788eeb71398c8e6dfda1d5bae161
).
Obviously the entire thing is not going to be reverted. Basically it is now up
to the caller to decide to call MessageBytes.setString manually as needed, to
avoid repeated conversions that could cause some GC. Since this is annoying
boilerplate code, maybe a new utility toString method could be introduced
(toStringType ?) and update callers as needed.
--
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