Code Review 6965924: java.net.HttpCookie using static SimpleDateFormat which is not thread safe

[Michael McMahon]

gustav trede wrote:
> On 19 August 2010 17:50, Chris Hegarty <chris.hegarty at oracle.com 
> <mailto:chris.hegarty at oracle.com>> wrote:
>
>     gustav trede wrote:
>
>
>         On 18 August 2010 13:22, Michael McMahon
>         <michael.x.mcmahon at oracle.com
>         <mailto:michael.x.mcmahon at oracle.com>
>         <mailto:michael.x.mcmahon at oracle.com
>         <mailto:michael.x.mcmahon at oracle.com>>> wrote:
>
>            gustav trede wrote:
>
>
>                On 18 August 2010 12:10, Chris Hegarty
>         <chris.hegarty at oracle.com <mailto:chris.hegarty at oracle.com>
>                <mailto:chris.hegarty at oracle.com
>         <mailto:chris.hegarty at oracle.com>>
>                <mailto:chris.hegarty at oracle.com
>         <mailto:chris.hegarty at oracle.com>
>                <mailto:chris.hegarty at oracle.com
>         <mailto:chris.hegarty at oracle.com>>>> wrote:
>
>                   Michael,
>
>                   java.net.HttpCookie uses static SimpleDateFormat
>         which is not
>                   thread safe. I think the best solution here is to
>         simply create
>                   local SimpleDateFormat as needed.
>
>                   Webrev:
>                  
>          http://cr.openjdk.java.net/~chegar/6965924/webrev.00/webrev/
>         <http://cr.openjdk.java.net/%7Echegar/6965924/webrev.00/webrev/>
>              
>          <http://cr.openjdk.java.net/%7Echegar/6965924/webrev.00/webrev/>
>                  
>         <http://cr.openjdk.java.net/%7Echegar/6965924/webrev.00/webrev/>
>
>
>
>                Why not use a threadlocal dateformater ?.
>
>            perhaps ...
>
>                For certain cases Its also viable to exploit the fact
>         that its
>                enough to generate a new value once per second for HTTP
>         timestamps.
>                Even if its not "needed", it would imo be nice if the
>         JDK code
>                itself could somehow act as reference / good examples
>         of how to
>                THINK(design) and implement.
>
>            I suspect you're looking at this from a server perspective.
>         This
>            code is involved with parsing
>            of incoming cookies. So, the generation of timestamps isn't
>         being
>            done here.
>
>         Yes i am quite server focused =), servers can act as http
>         clients too, different kind of intermediate logic etc.
>         Anyhow, my apologies for wasting your time with this not so
>         important issue, my brain just pings on code it finds strange /.
>
>
>     We use ThreadLocal to store the formatter for the HttpServer
>     implementation, com.sun.net.httpserver, see 6967684 "httpserver
>     using a non thread-safe SimpleDateFormat" [1]. The threads
>     creating the HTTP timestamps are part of an execurtorService and
>     will need a timestamp per response. These threads will not be used
>     for anything else, and are most probably going to handle many
>     requests. I think adding a threadLocal makes sense for these.
>
>     For HttpCookie, it is client side and a thread may only ever
>     handle a few cookies for its lifetime. I think adding the overhead
>     of three formatters may just be wasteful since the thread may
>     never do any more than a few HTTP requests.
>
>     Are you ok with this change?
>
> Sounds ok to me, considering how the current overall design and public 
> API is, the hands are a bit tied for radical changes.
>
> regards
>  gustav trede
Me too.

- Michael.