The servlet spec (see my previous question) guarantees that the same thread will execute all Filters and the associated Servlet. Given this, I do not see any use for passing data using HttpServletRequest.setAttribute if there is the option to use a ThreadLocal (assuming you clean up properly). I feel that there are two benefits to using ThreadLocal: type-safety and better performance because no string keys or maps are being used (except probably into a thread collection by (non-string) thread id).

Could someone please confirm if I am right so I can proceed with abandoning setAttribute?

I am not using JSPs ("if there is the option to use a ThreadLocal"), so this question is regarding using them for other reasons.
–
necromancerApr 10 '12 at 21:22

1

So where/how exactly are you looking to use these values? It sounds like you're just using global-ish variables instead of DI or regular-ol-argument passing.
–
Matt BallApr 10 '12 at 21:24

I primarily need to transport the User and EntityManager objects from the user and database Filters to the Servlet. I also find that these are frequently and unexpectedly needed in code further down the line and I am tempted to use them well beyond the Servlet (i. e. in nested code called by doGet). I feel there may be a better way for deeper code - suggestions? But I do not feel there is a better way for Filter to Servlet - suggestions? I have no JSP/JSF/other-framework dependencies and I am using pure Java with Servlets and Filters. I control all the code. Thanks!
–
necromancerApr 10 '12 at 21:31

1

EntityManager is very commonly obtained with DI - you should seriously consider that approach.
–
Matt BallApr 11 '12 at 3:29

4 Answers
4

Is ThreadLocal preferable to HttpServletRequest.setAttribute(“key”, “value”)?

Depends on the concrete functional requirement.

JSF, for example, stores the FacesContext in a ThreadLocal. This enables you to access all of the JSF artifacts, including the "raw" HttpServletRequest and HttpServletResponse anywhere in the code which is executed by the FacesServlet, such as managed beans. Most other Java based MVC frameworks follow the same example.

As per your comment,

I primarily need to transport the User and EntityManager objects from the user and database Filters to the Servlet. I also find that these are frequently and unexpectedly needed in code further down the line and I am tempted to use them well beyond the Servlet (i. e. in nested code called by doGet). I feel there may be a better way for deeper code - suggestions?

As to the User example for which I assume that this is a session attribute, I'd rather follow the same approach as JSF. Create a ThreadLocal<Context> where the Context is your custom wrapper class holding references to the current HttpServletRequest and maybe also HttpServletResponse so that you can access them anywhere down in your code. If necessary provide convenience methods to get among others the User directly from the Context class.

As to the EntityManager example, you could follow the same approach, but I'd personally not put it in the sameThreadLocal<Context>, but rather a different one. Or, better, just obtain it from JNDI in the service layer, which would allow you more finer grained control over transactions. In any case, please make absolutely sure that you handle the commit/close properly. Taking over the persistence and transaction management from the container should be done with extreme care. I'd really reconsider the aversion against using the existing and well-designed APIs/frameworks like EJB/JPA, otherwise you'll risk a complete waste of time of reinventing all the already-standardized APIs and stuffs.

See also:

Thank you for your answer. I prefer your approach, but for completeness, would you be kind enough to offer an opinion on the "singletons are bad" criticism of ThreadLocal? (all the other answers offer that criticism and prefer setAttribute so it would be nice to have that addressed if possible). thanks again!
–
necromancerApr 12 '12 at 5:30

A ThreadLocal is not a singleton. A singleton is a static class with lazily loaded static content which always returns the same for the remnant of application's lifetime.
–
BalusCApr 12 '12 at 12:48

Where you get the ThreadLocal from in this case is going to be a singleton (or otherwise static) class, since that is the mechanism of sharing the ThreadLocal between the filters and servlet. I would recommend not using such a singleton deeper in the code (unless you are passing an interface representation of the singleton down as a parameter). Furthermore, unless you are writing a framework which is encapsulating the underlying mechanisms, I would not use a ThreadLocal approach over attributes for performance reasons unless the application code is very tight such that this actually matters.
–
increment1Apr 12 '12 at 17:43

@increment1 - this has been a learning experience. @BalusC is right that it is not a singleton (see the javadoc, you can create multiple instances, and there's no lazy loading). It is not a static class either in the sense that there are no instances. ThreadLocal static methods are factory methods and create multiple instances. See the source code for get(): grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/… There are no global/static data structures; it internally uses the non-public threadLocals member of the Thread class.(continued)
–
necromancerApr 12 '12 at 19:30

So, no way there are any globals or singletons in the picture. There is already a per-thread Thread object available to any part of the code and ThreadLocal merely uses a field in per-thread Thread object to provide ThreadLocals. Therefore, none of the criticisms of singletons or globals apply. Please let me know if I am wrong, and if not, I am sure you will consider this to be the best answer. Thanks again for your own answer. It definitely helped me dig deeper and understand things better.
–
necromancerApr 12 '12 at 19:35

thread local works better if you are trying to set "global" variables for code "behind" whatever is dealing with the HttpServletRequest. If you're using JSP/JSF pages, or some other web UI component that reads from HttpServletRequest, then you'll end up having to pull the information out of the ThreadLocal yourself.
The 2 are not equivalent for most web programming.

Thanks. You do seem to imply request.setAttribute is better for the body of doGet or doPost, but I do not see why it is better to transport an object from a Filter to doGet using setAttribute/getAttribute? I feel that if the sender and receiver is entirely my own code with no dependencies then I should never use setAttribute even is the code is not "behind" whatever is dealing with HttpServletRequest. Am I missing something?
–
necromancerApr 10 '12 at 21:27

If it's you're own code, it probably doesn't matter much. You'll find it difficult to integrate with other libraries and code however.
–
Jim BarrowsApr 10 '12 at 21:32

You say it "doesn't matter much". What about my thought that ThreadLocal is better: "benefits to using ThreadLocal: type-safety and better performance because no string keys or maps are being used"? I definitely will not need to integrate it, or if I would, it is sufficiently contained to change it at that point.
–
necromancerApr 10 '12 at 21:35

Well, type safety is good. Performance, well, premature optimization is the root of all evil. Losing the ability to easily inter-operate in the future, not really worth the extra hassle in my opinion.
–
Jim BarrowsApr 10 '12 at 21:39

I can see why you're considering it, but I think you're falling into the premature optimization trap. ThreadLocals are essentially global variables - rarely a good design. The time-savings will be negligible. I guarantee this will never be the bottleneck in your server's throughput.

Using ThreadLocal may also give you problems if you want to start using asynchronous responses to some requests (e.g. to support long-polling) as the threading model is quite different.

type safety is my more important concern, and also switching from a string-keyed map to direct access is not in the category of premature optimization. kinda like moving from string representation of integers to byte representation would not be premature optimization. in summary, the main tradeoff is type-safety vs. "good design"(singleton, global, etc). the specific use case is transport data from filter to servlet and i am not able to see a concrete/specific demonstration of why it is not "good design". i appreciate good design but with code i give a bias to what fits specific use cases.
–
necromancerApr 11 '12 at 19:12

thanks for the answer, btw. you get your first upvote from me :)
–
necromancerApr 11 '12 at 19:12

Using a ThreadLocal in this fashion implies your are relying on some type of singleton to maintain shared state. Ultimately, this is usually considered poor design since it has a number of drawbacks, including difficulty in encapsulating functionality, being aware of dependencies, and an inability to swap (or mock) out functionality.

The performance overhead of using attributes or a session is probably worth it as compared to dealing with the less usual proposal of ThreadLocal variables in a singleton. However, this of course depends on your specific use case and project. Using singletons in servlets as a way to maintain application state is somewhat common due to the difficulty in sharing application state with servlets (difficult to inject the dependencies), but it is unusual for those objects singleton objects to maintain per user state (outside of EJBs).

If using a session or setting a container object as an attribute, you would only be dealing with a single fetch via String (which would be O(1)), and then a single case to your container type (which that has specific accessors for all of the values you want). Calling code further down the line should take parameters as appropriate and avoid as best you can using any type of global or singleton.

Ultimately, before considering a somewhat unusual (although clever) solution in the name of performance, always test the straightforward implementation first to see if its performance is adequate. The 2ns that might be saved here are most likely insignificant compared to the 20+ms being spent on db queries and tcp connection establishment.

I don't see the singleton. The object is per-thread for ThreadLocal, which has a 1-to-1 correspondence to per-request for set/getAttribute. The use-case is to transport the User object looked up by a Filter to the Servlet. In Filter I could say setAttribute("user", user); or ThreadStore.setUser(user); and in Servlet, correspondingly: User user = (User)getAttribute("user"); or ThreadStore.getUser() where ThreadStore is a convenience layer around ThreadLocal. I don't see the singleton.
–
necromancerApr 10 '12 at 22:14

@agksmehx The singleton I am referring to would be the object or static class where you are holding the globally accessible ThreadLocal variable (it must exist in some global staticly accessible manner for you to use it as indicated).
–
increment1Apr 10 '12 at 23:11

hmm... the example in the doc does have a singleton. i'm still not convinced if that's a bad thing or even if it is necessary. but your answer definitely earns my upvote :)
–
necromancerApr 10 '12 at 23:16

1

@agksmehx The String object itself caches its own hash code (and even String literals in quotes have an actual String object instance). What I would normally do in your case is have one object in the attributes or session that contains all the other objects, so I only have to worry about one cast. (e.g. UserContext context = (UserContext) request.getAttribute(USER_CONTEXT_KEY); int someInt = context.getSomeInt(); ). Deeper code then takes a UserContext object as a parameter (instead of passing request around or anything like that). Bonus points if UserContext is an interface.
–
increment1Apr 11 '12 at 20:37