-
Notifications
You must be signed in to change notification settings - Fork 7
Description
Currently it appears possible that ResourceRevision._create_from_stream() could create/leave a ResourceRevision row in the project database for a downloaded revision yet not finish downloading the revision's body to the project's "revisions" directory. See details for such a scenario below.
This is a problem because the resource revision would be left in an incompletely-downloaded state: with a ResourceRevision row saying it was downloaded successfully but no actual body in the "revisions" directory. This invalid state wouldn't be detected unless an attempt was made to actually read the resource revision, or a detailed comparison of the contents of the "revisions" directory was made against all ResourceRevision rows in the database, a time-consuming check to run.
Instead, let's find a way to download a ResourceRevision that could be interrupted without leaving the download in a corrupted state.
Scenario details:
- The scheduler thread (which is a daemon thread started in
start_scheduler_thread) starts downloading a URL revision by running a DownloadResourceBodyTask, which calls ResourceRevision._create_from_stream(). _create_from_stream() optimistically creates a ResourceRevision row in the project database and starts running the download of the URL. - The user presses Command-Q to trigger the Quit menuitem, via MainWindow._on_quit(), which calls set_is_quitting() and starts the process of closing the project.
- Project.close() calls Project._stop_scheduler() which signals the scheduler thread to cancel, using
self.root_task.cancel_tree(). - The scheduler thread doesn't observe the cancel request within the _SCHEDULER_JOIN_TIMEOUT (5.0 seconds) because it is busy running _create_from_stream(), which doesn't check the cancel status while it is downloading.
- Project.close() decides to proceed closing the project anyway, despite the scheduler thread not being stopped. In particular it calls Project._close_database() which closes the project database, preventing any further writes to ResourceRevision rows in the database.
- The MainWindow's wx.Frame finishes closing and MainWindow._on_close_frame() returns.
- There are no remaining top-level windows, so
app.MainLoop()in the_main2function ofmain.pyreturns. The non-headless version of the main loop is the most common version to be running. - The
_main2function continues running and observes thatis_quitting()is True, which causes_main2to return. - The callers of
_main2(_main1andmain) return and the foreground thread exits. - Now the only thread left running is the scheduler thread, but it is a daemon thread, so the thread doesn't prevent the process itself from trying to exit.
- The process starts trying to exit and calls the
on_atexithandler registered inmain.py. on_atexitfinishes by callingos._exit(exit_code), which immediately exits the process, despite the scheduler thread still running.- The final state of the project on-disk is:
- A ResourceRevision row in the project database exists which indicates the URL was downloaded successfully.
- A partially written revision body is written to the "tmp" (Project._TEMPORARY_DIRNAME) directory in the project's filesystem.
- When a future instance of the Crystal process is started and starts opening the project (as read-write, rather than readonly), Project._load() will clear any files inside the "tmp" directory, including the partially downloaded revision body. That future Crystal instance will interpret the existence of the ResourceRevision row in the project database to mean that the revision was successfully downloaded. Only if the user attempts to actually read the revision will Crystal discover that there is actually no revision body associated with the ResourceRevision row.
Impact:
- If the user attempts to quit Crystal while a large resource revision is being downloaded, which takes longer than 5.0 seconds to finish downloading under normal circumstances, Crystal will quit and leave the resource revision in a corrupt partially downloaded state.