Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thread safety of SimpleDateFormat #23

Closed
jarppe opened this issue Aug 6, 2013 · 2 comments
Closed

Thread safety of SimpleDateFormat #23

jarppe opened this issue Aug 6, 2013 · 2 comments

Comments

@jarppe
Copy link

jarppe commented Aug 6, 2013

It looks like the same SimpleDateFormat instance is shared between all log writers. I think that can cause some concurrency issues, because SimpleDateFormat is not thread safe.

@ptaoussanis
Copy link
Member

Hi Jarppe, thanks for the report! I've pushed an attempted soln. with v2.5.0 but I'm not certain if it actually fixes the problem since I'm not sure how to produce a detectable error with the old version. Any ideas?

EDIT: For reference, here's the current implementation: https://github.com/ptaoussanis/timbre/blob/master/src/taoensso/timbre.clj#L190

@jarppe
Copy link
Author

jarppe commented Aug 7, 2013

Hi Peter,

Yeah, I guess the only way to reveal threading problems is to stress test it with multiple threads and hope for the best.

Your fix looks good to me, should be thread safe now. In similar situations I have used the DateTimeFormat from JodaTime library as it's fully thread safe, but that would mean a new dependency.

-jarppe

On 2013/07/08, at 08:52, Peter Taoussanis notifications@github.com wrote:

Hi Jarppe, thanks for the report! I've pushed an attempted soln. with v2.5.0 but I'm not certain if it actually fixes the problem since I'm not sure how I'd produce a detectable actual error with the old version. Any ideas?


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants