Add a lock around retrieving upstream files

Created on 26 May 2022, over 2 years ago
Updated 17 January 2023, almost 2 years ago

Problem/Motivation

When retrieving an upstream files, it's possible for multiple requests to require the same image or file concurrently. As is, a warning will be triggered and a `Location` header sent to try again.

Unfortunately, this is confusing when debugging other issues, because it looks like something is wrong with the download code or the underlying file system. As well, it's possible for the upstream system to start denying requests (or even all of them!) if it thinks it's being attacked.

Since Stage File Proxy is nearly always used on locals and development environments, holding a PHP process open with a lock for an additional second has few downsides and avoids triggering file races at all.

Steps to reproduce

Fetch an image that needs to be downloaded from the origin with multiple processes concurrently.

Proposed resolution

Unfortunately \Drupal\stage_file_proxy\FetchManagerInterface declares a constructor in it's interface, which makes it hard to inject additional services. As such, we'll need to:

  1. Deprecate \Drupal\stage_file_proxy\FetchManagerInterface and create a new interface, along with an underlying class. FetchManager can decorate the new class.
  2. Deprecate \Drupal\stage_file_proxy\EventSubscriber\ProxySubscriber so we can change it's protected $manager property.
  3. Write new classes that support injecting the lock service.
🐛 Bug report
Status

Fixed

Version

2.0

Component

Code

Created by

🇨🇦Canada deviantintegral

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Status changed to Needs review almost 2 years ago
  • Status changed to Fixed almost 2 years ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024