@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator

When file transforms race and lose, accept defeat gracefully

Summary: Fixes T8277. Transforming files can race; resolve the race after we lose.

Test Plan:
- Added `sleep(10)` near the bottom of the transform controller.
- Transformed a file in two browser windows at the same time; got something like this (exception corresponds to the loser of the race):

{F412526}

- Applied patch.
- Repeated process, got this:

{F412527}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8277

Differential Revision: https://secure.phabricator.com/D12965

+25 -8
+25 -8
src/applications/files/controller/PhabricatorFileTransformController.php
··· 30 30 } 31 31 32 32 $transform = $request->getURIData('transform'); 33 - $xform = id(new PhabricatorTransformedFile()) 34 - ->loadOneWhere( 35 - 'originalPHID = %s AND transform = %s', 36 - $source_phid, 37 - $transform); 33 + $xform = $this->loadTransform($source_phid, $transform); 38 34 39 35 if ($xform) { 40 36 if ($is_regenerate) { ··· 80 76 $xform = id(new PhabricatorTransformedFile()) 81 77 ->setOriginalPHID($source_phid) 82 78 ->setTransform($transform) 83 - ->setTransformedPHID($xformed_file->getPHID()) 84 - ->save(); 79 + ->setTransformedPHID($xformed_file->getPHID()); 80 + 81 + try { 82 + $xform->save(); 83 + } catch (AphrontDuplicateKeyQueryException $ex) { 84 + // If we collide when saving, we've raced another endpoint which was 85 + // transforming the same file. Just throw our work away and use that 86 + // transform instead. 87 + $this->destroyTransform($xform); 88 + $xform = $this->loadTransform($source_phid, $transform); 89 + if (!$xform) { 90 + return new Aphront404Response(); 91 + } 92 + } 85 93 86 94 return $this->buildTransformedFileResponse($xform); 87 95 } ··· 113 121 $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); 114 122 115 123 if (!$file) { 116 - $xform->delete(); 124 + if ($xform->getID()) { 125 + $xform->delete(); 126 + } 117 127 } else { 118 128 $engine->destroyObject($file); 119 129 } 120 130 121 131 unset($unguarded); 132 + } 133 + 134 + private function loadTransform($source_phid, $transform) { 135 + return id(new PhabricatorTransformedFile())->loadOneWhere( 136 + 'originalPHID = %s AND transform = %s', 137 + $source_phid, 138 + $transform); 122 139 } 123 140 124 141 }