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

Race condition in File Conversion API

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 3.3.2
    • Fix Version/s: None
    • Component/s: Files API
    • Affected Branches:
      MOODLE_33_STABLE

      Description

      When using an async conversion plugin, there is an area of the file conversion API that can cause a race condition (and in my test environment, it happens to happen every time...).

      So basically there are two flows that happen here. One for the client side 'is this document converted' looks like this (really dumbed down to the important bits):

      1. converter::start_conversion() calls conversion::get_conversions_for_file() to see if there are any existing conversions
      2. conversion::get_conversions_for_file() does a large multi-join and UNION query that looks for two things (in one query). On a large system, this may take a while:
        • Any records in mdl_file_conversion for the source file id and the dest file format
        • Find any existing conversioned files that happen to not have a record in mdl_file_conversion
      3. Anything found in step 2 is returned to converter::start_conversion()
      4. If more than 1 record is returned, then they are all deleted and it starts a new conversion.
      5. If just 1 record is returned, then converter::poll_conversion is called
      6. converter::poll_conversion will always call $conversion->update() if the conversion is in progress.

      From on the async converter side, we:

      1. Mark the conversion as started in the DB (using an internal data field)
      2. Do the actual conversion (this may take some time)
      3. Load the converted file into the conversion (conversion::store_destfile_from_path())
      4. Save to the database

      Now because of the particular timing, this is the order of what happens:

      1. Client Thread: First conversion request comes in from client, queued up, and returned as IN_PROGRESS
      2. Converter Thread: Converter picks up task and marks it started
      3. Client Thread: Second client request comes in, asking for the conversion
      4. Client Thread: conversion::get_conversions_for_file() starts its' big query
      5. Converter Thread: Completes conversion, ingests it, and saves destfileid and the completion status to the DB
      6. Client Thread: Big query returns, having only found the 1 conversion without the destfileid set, and also doesn't find the completed file (since these steps both happened while the query was in progress).
      7. Client Thread: converter::poll_conversion calls update() on the conversion object, which saves it to the DB. This overwrites the destfileid to blank (and resets the status), because the local object is blank, even though it has been updated in the database.

      From then on, for each subsequent query (once a second in the assign interface), the big query finds 2 records, one for the conversion that it feels is 'incomplete', and one for the already converted file. This causes a force reset to happen, which deletes the waiting conversions and submits a new one, and this just happens over and over again, for as long as you leave the browser open.

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              emerrill Eric Merrill
              Participants:
              Component watchers:
              Matteo Scaramuccia, Jake Dallimore, Jun Pataleta, Ryan Wyllie
            • Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated: