Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-35667

Potential infinite loop in grade_category::apply_limit_rules()

          Details

          • Type: Bug
          • Status: Closed
          • Priority: Major
          • Resolution: Fixed
          • Affects Version/s: 2.2.5
          • Fix Version/s: 2.2.6, 2.3.3
          • Component/s: Gradebook
          • Labels:

            Description

            I've spotted a possible infinite loop introduced by MDL-13629 to do with droplow and keephigh and extra credit grade items.

              Gliffy Diagrams

                Attachments

                  Issue Links

                  is a regression caused by

                  Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-13629 Gradebook: Unexpected behavior in selection of lowest grade to drop and resultant category calculation

                  • Critical - Crashes server, loss of data, severe memory leak
                  • Closed

                    Activity

                    Ascending order - Click to sort in descending order
                    Hide
                    Permalink
                    andyjdavis Andrew Davis added a comment -

                    Requesting peer review. Assuming that goes fine Ill backport this to 2.3 and 2.2.

                    Show
                    andyjdavis Andrew Davis added a comment - Requesting peer review. Assuming that goes fine Ill backport this to 2.3 and 2.2.
                    Hide
                    Permalink
                    poltawski Dan Poltawski added a comment -

                    Makes sense to me Andrew.

                    If you were feeling keen you add more to the the unit tests which exposes this bug (benefit of not introducing the same problem in the future).

                    Show
                    poltawski Dan Poltawski added a comment - Makes sense to me Andrew. If you were feeling keen you add more to the the unit tests which exposes this bug (benefit of not introducing the same problem in the future).
                    Hide
                    Permalink
                    andyjdavis Andrew Davis added a comment -

                    I've added a new test that demonstrated the bug. The new test is in a second commit. It can only go into master and 2.3, not 2.2. 2.2 gets the bug fix but not the test due to changes in the unit testing framework.

                    Peer review part 2

                    Show
                    andyjdavis Andrew Davis added a comment - I've added a new test that demonstrated the bug. The new test is in a second commit. It can only go into master and 2.3, not 2.2. 2.2 gets the bug fix but not the test due to changes in the unit testing framework. Peer review part 2
                    Hide
                    Permalink
                    poltawski Dan Poltawski added a comment -

                    Looks great, thanks Andrew!

                    Show
                    poltawski Dan Poltawski added a comment - Looks great, thanks Andrew!
                    Hide
                    Permalink
                    eric.eisenhart Eric Eisenhart added a comment - - edited

                    This definitely affects 2.3.2. Ran into it during the cron job, "Looking for quiz overdue quiz attempts between [yesterday 11:05am] and [about right now]", and just hung there. Tracked down with xdebug stuff and found it was hanging in this infinite loop. Applying the 2.3 diff fixed it.

                    Show
                    eric.eisenhart Eric Eisenhart added a comment - - edited This definitely affects 2.3.2. Ran into it during the cron job, "Looking for quiz overdue quiz attempts between [yesterday 11:05am] and [about right now] ", and just hung there. Tracked down with xdebug stuff and found it was hanging in this infinite loop. Applying the 2.3 diff fixed it.
                    Hide
                    Permalink
                    samhemelryk Sam Hemelryk added a comment -

                    Thanks Andrew, this has been integrated now

                    Show
                    samhemelryk Sam Hemelryk added a comment - Thanks Andrew, this has been integrated now
                    Hide
                    Permalink
                    abgreeve Adrian Greeve added a comment -

                    Tested on the 2.2, 2.3 and master integration branches.
                    I repeated the tests for MDL-13629 - No problems there.
                    I ran the unit tests as well - No problems.
                    Test passed.

                    Show
                    abgreeve Adrian Greeve added a comment - Tested on the 2.2, 2.3 and master integration branches. I repeated the tests for MDL-13629 - No problems there. I ran the unit tests as well - No problems. Test passed.
                    Hide
                    Permalink
                    stronk7 Eloy Lafuente (stronk7) added a comment -

                    From somewhere within the clouds...

                    Congrats, this has been sent upstream and is now part of Moodle (your favorite LMS platform). Many thanks for your awesome collaboration!

                    Ciao

                    Show
                    stronk7 Eloy Lafuente (stronk7) added a comment - From somewhere within the clouds... Congrats, this has been sent upstream and is now part of Moodle (your favorite LMS platform). Many thanks for your awesome collaboration! Ciao

                      People

                      • Votes:
                        2 Vote for this issue
                        Watchers:
                        3 Start watching this issue

                        Dates

                        • Created:
                          Updated:
                          Resolved:
                          Fix Release Date:
                          12/Nov/12