eclipse-ee4j / grizzly

Grizzly
https://eclipse-ee4j.github.io/grizzly
Other
150 stars 70 forks source link

Don't use StringBuffer this way ... #272

Closed glassfishrobot closed 14 years ago

glassfishrobot commented 16 years ago

Grizzly misues StringBuffer at several places. In some public methods of certain classes as well, StringBuffer is mysteriously required as a method argument. JSONParser and ServerCookie classes are examples of this. At one place in JSONParser class, a method accepts a StringBuffer (which is already synchronized) and then adds a synchronized block on passed instance of StringBuffer!

Enough has been said about misuse of (and performance problems with) using StringBuffers when StringBuilders are appropriate. For example, all the local variables must be StringBuilders. It is in fact much better to use the old styled (and criticized) string concatenation operator (str += s) because javac will optimize away and use StringBuilder there. Using StringBuffer in such places however defeats the purpose.

See: http://jeremymanson.blogspot.com/2008/08/dont-use-stringbuffer.html for more detailed treatment of this.

Attached is a patch that carefully replaces use of StringBuffer with StringBuilder. IMO, it is safe to apply this patch. After applying the patch, keep the bug open to scrutinize the remaining instances of StringBuffer.

Environment

Operating System: All Platform: Macintosh

Affected Versions

[1.9.22]

glassfishrobot commented 6 years ago
glassfishrobot commented 16 years ago

@glassfishrobot Commented Reported by km@java.net

glassfishrobot commented 16 years ago

@glassfishrobot Commented km@java.net said: Created an attachment (id=20) replace StringBuffer with StringBuilder

glassfishrobot commented 16 years ago

@glassfishrobot Commented jfarcand@java.net said: Thanks Kedar!!! Both classes are from (1) Jetty or (2) Tomcat This is great to fix them!

glassfishrobot commented 16 years ago

@glassfishrobot Commented jfarcand@java.net said: Change category as enhancement. Working on applying the patch.

glassfishrobot commented 16 years ago

@glassfishrobot Commented jfarcand@java.net said: Fixed. MANY thanks!

Sending modules/cometd/src/main/java/com/sun/grizzly/cometd/CometdNotificationHandler.java Sending modules/cometd/src/main/java/com/sun/grizzly/cometd/bayeux/Data.java Sending modules/cometd/src/main/java/com/sun/grizzly/cometd/bayeux/Ext.java Sending modules/grizzly/src/main/java/com/sun/grizzly/connectioncache/client/CacheableConnectorHandler.java Sending modules/grizzly/src/main/java/com/sun/grizzly/util/SSLUtils.java Sending modules/http/src/main/java/com/sun/grizzly/http/HtmlHelper.java Sending modules/http-utils/src/main/java/com/sun/grizzly/tcp/Response.java Sending modules/http-utils/src/main/java/com/sun/grizzly/tcp/http11/ClientAbortException.java Sending modules/http-utils/src/main/java/com/sun/grizzly/tcp/http11/GrizzlyReader.java Sending modules/http-utils/src/main/java/com/sun/grizzly/tcp/http11/GrizzlyRequest.java Sending modules/http-utils/src/main/java/com/sun/grizzly/tcp/http11/GrizzlyResponse.java Sending modules/http-utils/src/main/java/com/sun/grizzly/util/IntrospectionUtils.java Sending modules/http-utils/src/main/java/com/sun/grizzly/util/buf/Base64.java Sending modules/http-utils/src/main/java/com/sun/grizzly/util/buf/HexUtils.java Sending modules/http-utils/src/main/java/com/sun/grizzly/util/buf/UDecoder.java Sending modules/http-utils/src/main/java/com/sun/grizzly/util/http/HttpMessages.java Sending modules/http-utils/src/main/java/com/sun/grizzly/util/http/Parameters.java Sending modules/http-utils/src/main/java/com/sun/grizzly/util/http/ServerCookie.java Sending modules/http-utils/src/main/java/com/sun/grizzly/util/http/mapper/Mapper.java Sending modules/http-utils/src/main/java/com/sun/grizzly/util/net/URL.java Sending modules/http-utils/src/main/java/com/sun/grizzly/util/net/jsse/JSSESupport.java Sending modules/http-utils/src/main/java/com/sun/grizzly/util/res/StringManager.java Sending pom.xml Transmitting file data ....................... Committed revision 1667.

glassfishrobot commented 16 years ago

@glassfishrobot Commented File: dontusestringbuffer.txt Attached By: km@java.net

glassfishrobot commented 16 years ago

@glassfishrobot Commented Was assigned to jfarcand@java.net

glassfishrobot commented 7 years ago

@glassfishrobot Commented This issue was imported from java.net JIRA GRIZZLY-272

glassfishrobot commented 14 years ago

@glassfishrobot Commented Marked as fixed on Tuesday, December 15th 2009, 6:27:38 pm