JAX-RS / Jackson Thread Safety w/ThreadLocal

[apemberton]

This is a follow-up to the forum post here: http://jackson-users.ning.com/forum/topics/jax-rs-jacksonjsonprovider-thread-safety-issues?commentId=5286555%3AComment%3A29197

Provided a very simple ThreadLocal-based solution for accessing the in context JAX-RS ObjectWriter.

[cowtowncoder]

Just to be sure: the point here would be to make ObjectWriter be accessible via ThreadLocal? That sounds reasonable. But one thing is that mapping should be cleared afterwards.

Also: I think it would make sense to add this as a feature; I know that it is not a big change, but I suspect some users could be uncomfortable adding more things in context.

[apemberton]

Thanks for the quick response, @cowtowncoder!

Agree - I am not 100% comfortable with TheadLocal implementation myself, but any cleaner alternative would involve a fairly significant refactor to com.fasterxml.jackson.jaxrs.base.ProviderBase.

Thoughts?

[cowtowncoder]

I am actually fine with ThreadLocal access itself (knowing the challenges, and as per earlier discussion). What I meant was that it'd be good to clean after call, and make this optional.

But I can make those changes myself afterwards; so let me think about this for a bit, and go ahead.

One more thing I need to ask is Contributor License Agreement. It's the standard thing we have to ask from all (new) contributors:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

and one is enough for all contributions. Would it be possible to fill that out, scan and email to us (info at fasterxml.com)? It can be personal one (leaving company empty), or employer-specific one; most contributors do former. CLA itself is from Sun so hopefully there is nothing unusual there -- we just need it to distribute code.

Apologies if I have already gotten one: list of CLAs we have is long and sometimes I forget who has already sent one. :)

[apemberton]

Oh - I understand now...

• Regarding cleaning after call - yes - it currently does that and is indeed optional. By calling ObjectWriterThreadLocal.mergeAndUnset(endpoint.getWriter()); in ProviderBase, this will pull a Writer from ThreadLocal if the client (eg: a web app) put one there, but default to the existing endpoint.getWriter call otherwise

[cowtowncoder]

Hmmh. Ok, after re-reading the code I realized I hadn't quite understood the order of things; and had assumed reverse relationship of provider exposing Writer (and/or Reader). But that wouldn't work with filters, so code makes sense.

I think I can improve on merging part a bit: I think it is possible to actually assume thatObjectWriter (and I can extend this to ObjectReader as well) from ThreadLocal is initialized with proper settings, and only decorate it with overrides from endpoint configuration.
I can use pull request as the base, the end result will look slightly different, but the idea is the same.

[cowtowncoder]

Hmmh. Ok, after reworking this and checking it in, I realized that I have one fundamental flaw in my thinking.
Problem being that whereas ObjectReader / ObjectWriter instances created for end points are static, after initial lazy construction, filter-injected reader/writer is dynamic, per call. So the direction is now wrong.

But trying to merge readers and writers is nasty business; so what I am wondering, rather, is whether we could and should use some other abstraction. For example, a configuration object used for (re)configuring what endpoint has. The problem is that filter does not have access to end point (to obtain baseline to modify); and endpoint only gets control once filter is done.

[cowtowncoder]

Ok. I think I have a better idea. Instead of trying to build a configuration object of some sort, it is better to have a "Reader modifier" and "Writer modifier" objects that can be registered on per-request basis. These take fully set up ObjectReader / ObjectWriter (as per endpoint config), and return instance of same type; either one passed in as-is, or one modified accordingly. This allows proper (re)configuration based on needs of caller.

Registration itself can be passed using ThreadLocal as suggested.

Would this make sense?

[cowtowncoder]

Created #32 for what I think is the gisat of request here, implemented. Please let me know if that solution would work for your use case?