adzap / timeliness

Fast date/time parsing for the control freak.
http://github.com/adzap/timeliness
MIT License
224 stars 28 forks source link

Mention that Timeliness is not thread safe. #21

Closed andruby closed 5 years ago

andruby commented 8 years ago

Reproducable with:

eu_date = "30/06/2016"
us_date = "06/30/2016"
threads = []
threads << Thread.new { Timeliness.use_euro_formats; 10_000.times { Timeliness.parse(eu_date).to_date } }
threads << Thread.new { Timeliness.use_us_formats; 10_000.times { Timeliness.parse(us_date).to_date } }
threads.each { |thr| thr.join }

This will raise a NoMethodError: undefined method 'to_date' for nil:NilClass because it parsed one of the dates in the wrong US/EU format.

I suggest we at least mention this in the README. We bumped into this using Timeliness inside sidekiq workers.

adzap commented 8 years ago

Ah yes fair enough. The gem is poorly designed for this usage. Though I am keen to resolve this. It shouldn't be too hard.

andruby commented 8 years ago

I'll take a shot at making it threadsafe on Monday.

I was thinking to store the class variables from Timeliness::Definitions in Thread.current[].

Is that the same approach you were considering?

On 02 Jul 2016, at 03:54, Adam Meehan notifications@github.com wrote:

Ah yes fair enough. The gem is poorly designed for this usage. Though I am keen to resolve this. It shouldn't be too hard.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

dgilperez commented 8 years ago

Any news? Thanks!

andruby commented 8 years ago

Hi @dgilperez. I thought I had already opened a PR, but it doesn't seem that way. Here's the branch we use in production with sidekiq threads: https://github.com/andruby/timeliness/tree/multithread. I'll try to open a PR when I have time.

lawso017 commented 7 years ago

@adzap would you have a moment to merge @andruby 's multithread changes into Timeliness? Looking forward to the ability to adjust US/ROW date parsing on a per-request basis, already using your excellent validate gem but would need an updaetd timeliness gem with thread local configuration info.