listen uses Time.now for time intervals; should use Monotonic tick count

[ColinDKelley]

Current State

The 'listen gem uses the Time.now clock for measuring time intervals. For example, from base.rb:

      def _wait_until_events_calm_down
        loop do
          now = _timestamp

          # Assure there's at least latency between callbacks to allow
          # for accumulating changes
          diff = _deadline - now
          break if diff <= 0

          # give events a bit of time to accumulate so they can be
          # compressed/optimized
          _sleep(diff)
        end
      end

...
      def _timestamp
        config.timestamp
      end
...
      def timestamp
        Time.now.to_f
      end

This is a bug since the Time.now timer may be updated at any time, forward or backward. Most commonly by ntp.

Desired State

The 'listen gem should use the Monotonic tick count instead for time intervals. There are several gems that encapsulate this, including https://github.com/Freaky/monotime, https://github.com/invoca/monotonic_tick_count. Perhaps best for portability and minimal coupling, concurrent-ruby has a simple encapsulation as monotonic_time.

Steps to Reproduce

I didn't try, but you could reproduce the bug by adjusting the system clock while listen is running. If you adjusted the clock forward by, say, 1 second, you could trick the _wait_until_events_calm_down method into thinking it didn't need to wait at all, thus leading to duplicate events.

[ColinDKelley]

@ioquatix Curious to get your opinion here.

[ioquatix]

Just use the Ruby interface don't pull in a dependency for it.

[ColinDKelley]

Sounds good to avoid dependencies. I wonder if we'll have trouble with some Ruby versions not supporting Process::CLOCK_MONOTONIC? concurrent-ruby has code that adapts for that not being defined in MRI, and a special case that talks to the JVM in JRuby. I suppose that could just be outdated? If we do hit it, we could copy that if/elsif/else logic in here, I suppose.

[ioquatix]

You can ignore any version that doesn't support it, they should all be EOL.

[ColinDKelley]

Sounds good. BTW this is a minor bug in the scheme of things, so I'd like to release v3.3.0 before fixing this.

[ioquatix]

Sure, if you are happy with that, I am happy with it. It's no worse than it currently is.

[ColinDKelley]

@ioquatix

You can ignore any version that doesn't support it, they should all be EOL.

From reading the documentation, it seems like those constants are more OS-specific than Ruby-version-specific.

So just in case, I wrote the code to be adaptive and prefer Process::CLOCK_MONOTONIC, then Process::CLOCK_MONOTONIC_RAW, and finally to fall back to Time.now.to_f.

[ioquatix]

I have not seen a platform where Process::CLOCK_MONOTONIC is not supported. But if you think it's for the best to be compatible, it's acceptable.