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

Infinite loop when invoking #first #549

Open
jorgemanrubia opened this issue Jan 19, 2024 · 1 comment
Open

Infinite loop when invoking #first #549

jorgemanrubia opened this issue Jan 19, 2024 · 1 comment

Comments

@jorgemanrubia
Copy link

jorgemanrubia commented Jan 19, 2024

This enters an infinite loop:

schedule = IceCube::Schedule.new(Time.current) do |schedule|
  schedule.add_recurrence_rule IceCube::IcalParser.rule_from_ical("FREQ=YEARLY;INTERVAL=1;BYMONTHDAY=31;BYMONTH=2")
end
schedule.first(2) # hangs!

We have patched this internally with:

module ValidatedRuleWithInfiniteRecursionGuard
  private
    MAX_LOOPS = 10_000

    def find_acceptable_time_before(boundary)
      count = 0
      until finds_acceptable_time? || count > MAX_LOOPS
        return false if past_closing_time?(boundary)
        count += 1
      end
      true
    end
end

IceCube::ValidatedRule.prepend(ValidatedRuleWithInfiniteRecursionGuard)

I'm happy to submit a fix to the library. It could be the specific patch suggested above, but the several loop and until usages inside the library, combined with the complex logic, looks like a liability. I think a better patch would be to replace the current loop and until usages with safe_ versions that control a max number of iterations to prevent these infinite loops by design. For example:

safe_loop do # m = 10_000 or whatever by default, and can be passed by parameter 
end

What do you think?

Similar to #469

@pacso
Copy link
Collaborator

pacso commented May 10, 2024

I think it's probably better to ensure the rule is valid, rather than break out of an infinite loop. Although, it would obviously be better to gracefully handle an infinite loop than crash.

For example, #554 will fix the issue raised in #469, but does not handle infinite loops.

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