| Index: branches/FileBackend/phase3/includes/filerepo/backend/FileBackendMultiWrite.php |
| — | — | @@ -83,8 +83,8 @@ |
| 84 | 84 | } |
| 85 | 85 | } |
| 86 | 86 | |
| 87 | | - // Try to lock those files... |
| 88 | | - $status->merge( $this->lockFiles( $filesToLock ) ); |
| | 87 | + // Try to lock those files for the scope of this function... |
| | 88 | + $scopedLock = $this->getScopedLock( $filesToLock, $status ); |
| 89 | 89 | if ( !$status->isOK() ) { |
| 90 | 90 | return $status; // abort |
| 91 | 91 | } |
| — | — | @@ -105,7 +105,6 @@ |
| 106 | 106 | ++$status->failCount; |
| 107 | 107 | $status->success[$index] = false; |
| 108 | 108 | } else { |
| 109 | | - $status->merge( $this->unlockFiles( $filesToLock ) ); |
| 110 | 109 | return $status; |
| 111 | 110 | } |
| 112 | 111 | } |
| — | — | @@ -132,7 +131,6 @@ |
| 133 | 132 | } |
| 134 | 133 | $pos--; |
| 135 | 134 | } |
| 136 | | - $status->merge( $this->unlockFiles( $filesToLock ) ); |
| 137 | 135 | return $status; |
| 138 | 136 | } |
| 139 | 137 | } |
| — | — | @@ -154,12 +152,9 @@ |
| 155 | 153 | $status->merge( $subStatus ); |
| 156 | 154 | } |
| 157 | 155 | |
| 158 | | - // Unlock all the files |
| 159 | | - $status->merge( $this->unlockFiles( $filesToLock ) ); |
| | 156 | + // Make sure status is OK, despite any finish() fatals |
| | 157 | + $status->setResult( true, $status->value ); |
| 160 | 158 | |
| 161 | | - // Make sure status is OK, despite any finish() or unlockFiles() fatals |
| 162 | | - $status->setResult( true ); |
| 163 | | - |
| 164 | 159 | return $status; |
| 165 | 160 | } |
| 166 | 161 | |
| — | — | @@ -232,8 +227,12 @@ |
| 233 | 228 | // Pass isOK() despite fatals from other backends |
| 234 | 229 | $status->setResult( true ); |
| 235 | 230 | return $status; |
| 236 | | - } elseif ( headers_sent() ) { |
| 237 | | - return $status; // died mid-stream...so this is already fubar |
| | 231 | + } else { // failure |
| | 232 | + if ( headers_sent() ) { |
| | 233 | + return $status; // died mid-stream...so this is already fubar |
| | 234 | + } elseif ( strval( ob_get_contents() ) !== '' ) { |
| | 235 | + ob_clean(); // output was buffered but not sent; clear it |
| | 236 | + } |
| 238 | 237 | } |
| 239 | 238 | } |
| 240 | 239 | return $status; |
| Index: branches/FileBackend/phase3/includes/filerepo/backend/LockManager.php |
| — | — | @@ -251,13 +251,9 @@ |
| 252 | 252 | } |
| 253 | 253 | |
| 254 | 254 | function __destruct() { |
| 255 | | - // Make sure remaining files get cleared for sanity |
| 256 | | - foreach ( $this->handles as $key => $locks ) { |
| 257 | | - foreach ( $locks as $type => $handle ) { |
| 258 | | - flock( $handle, LOCK_UN ); // PHP 5.3 will not do this automatically |
| 259 | | - fclose( $handle ); |
| 260 | | - } |
| 261 | | - unlink( $this->getLockPath( $key ) ); |
| | 255 | + // Make sure remaining locks get cleared for sanity |
| | 256 | + foreach ( $this->locksHeld as $key => $locks ) { |
| | 257 | + $this->doSingleUnlock( $key, 0 ); |
| 262 | 258 | } |
| 263 | 259 | } |
| 264 | 260 | } |
| — | — | @@ -692,6 +688,11 @@ |
| 693 | 689 | $prefix = substr( $key, 0, 2 ); // first 2 hex chars (8 bits) |
| 694 | 690 | return intval( base_convert( $prefix, 16, 10 ) ) % count( $this->dbsByBucket ); |
| 695 | 691 | } |
| | 692 | + |
| | 693 | + function __destruct() { |
| | 694 | + // Make sure remaining locks get cleared for sanity |
| | 695 | + $this->finishLockTransactions(); |
| | 696 | + } |
| 696 | 697 | } |
| 697 | 698 | |
| 698 | 699 | /** |
| Index: branches/FileBackend/phase3/includes/filerepo/backend/FileBackend.php |
| — | — | @@ -27,7 +27,8 @@ |
| 28 | 28 | |
| 29 | 29 | /** |
| 30 | 30 | * Build a new object from configuration. |
| 31 | | - * This should only be called from within FileRepo classes. |
| | 31 | + * This should only be called from within FileBackendGroup. |
| | 32 | + * |
| 32 | 33 | * $config includes: |
| 33 | 34 | * 'name' : The name of this backend |
| 34 | 35 | * 'wikiId' : Prefix to container names that is unique to this wiki |
| — | — | @@ -44,9 +45,10 @@ |
| 45 | 46 | } |
| 46 | 47 | |
| 47 | 48 | /** |
| | 49 | + * Get the unique backend name. |
| | 50 | + * |
| 48 | 51 | * We may have multiple different backends of the same type. |
| 49 | 52 | * For example, we can have two Swift backends using different proxies. |
| 50 | | - * All backend instances must have unique names. |
| 51 | 53 | * |
| 52 | 54 | * @return string |
| 53 | 55 | */ |
| — | — | @@ -93,7 +95,7 @@ |
| 94 | 96 | /** |
| 95 | 97 | * Prepare a storage path for usage. This will create containers |
| 96 | 98 | * that don't yet exists or, on FS backends, create parent directories. |
| 97 | | - * Do not call this function from places outside FileBackend and FileOp. |
| | 99 | + * |
| 98 | 100 | * $params include: |
| 99 | 101 | * dir : storage directory |
| 100 | 102 | * |
| — | — | @@ -104,8 +106,10 @@ |
| 105 | 107 | |
| 106 | 108 | /** |
| 107 | 109 | * Take measures to block web access to a directory. |
| | 110 | + * In backends like Swift, this might restrict container |
| | 111 | + * access to backend user that represents user web request. |
| 108 | 112 | * This is not guaranteed to actually do anything. |
| 109 | | - * Do not call this function from places outside FileBackend and FileOp. |
| | 113 | + * |
| 110 | 114 | * $params include: |
| 111 | 115 | * dir : storage directory |
| 112 | 116 | * noAccess : try to deny file access |
| — | — | @@ -119,7 +123,7 @@ |
| 120 | 124 | /** |
| 121 | 125 | * Clean up an empty storage directory. |
| 122 | 126 | * On FS backends, the directory will be deleted. Others may do nothing. |
| 123 | | - * Do not call this function from places outside FileBackend and FileOp. |
| | 127 | + * |
| 124 | 128 | * $params include: |
| 125 | 129 | * dir : storage directory |
| 126 | 130 | * |
| — | — | @@ -129,8 +133,8 @@ |
| 130 | 134 | abstract public function clean( array $params ); |
| 131 | 135 | |
| 132 | 136 | /** |
| 133 | | - * Check if a file exits at a storage path in the backend. |
| 134 | | - * Do not call this function from places outside FileBackend and FileOp. |
| | 137 | + * Check if a file exists at a storage path in the backend. |
| | 138 | + * |
| 135 | 139 | * $params include: |
| 136 | 140 | * src : source storage path |
| 137 | 141 | * |
| — | — | @@ -142,6 +146,7 @@ |
| 143 | 147 | /** |
| 144 | 148 | * Get a hash of the file at a storage path in the backend. |
| 145 | 149 | * Typically this will be a SHA-1 hash, MD5 hash, or something similar. |
| | 150 | + * |
| 146 | 151 | * $params include: |
| 147 | 152 | * src : source storage path |
| 148 | 153 | * |
| — | — | @@ -153,12 +158,13 @@ |
| 154 | 159 | /** |
| 155 | 160 | * Get the format of the hash that getFileHash() uses |
| 156 | 161 | * |
| 157 | | - * @return string (md5, sha1, unknown, ...) |
| | 162 | + * @return string (md5, sha1, internal, ...) |
| 158 | 163 | */ |
| 159 | 164 | abstract public function getHashType(); |
| 160 | 165 | |
| 161 | 166 | /** |
| 162 | 167 | * Get the last-modified timestamp of the file at a storage path. |
| | 168 | + * |
| 163 | 169 | * $params include: |
| 164 | 170 | * src : source storage path |
| 165 | 171 | * |
| — | — | @@ -170,6 +176,7 @@ |
| 171 | 177 | /** |
| 172 | 178 | * Get the properties of the file at a storage path in the backend. |
| 173 | 179 | * Returns FSFile::placeholderProps() on failure. |
| | 180 | + * |
| 174 | 181 | * $params include: |
| 175 | 182 | * src : source storage path |
| 176 | 183 | * |
| — | — | @@ -183,6 +190,7 @@ |
| 184 | 191 | * Appropriate HTTP headers (Status, Content-Type, Content-Length) |
| 185 | 192 | * must be sent if streaming began, while none should be sent otherwise. |
| 186 | 193 | * Implementations should flush the output buffer before sending data. |
| | 194 | + * |
| 187 | 195 | * $params include: |
| 188 | 196 | * src : source storage path |
| 189 | 197 | * headers : additional HTTP headers to send on success |
| — | — | @@ -198,6 +206,7 @@ |
| 199 | 207 | * the container should be listed. If of the form "mwstore://container/dir", |
| 200 | 208 | * then all items under that container directory should be listed. |
| 201 | 209 | * Results should be storage paths relative to the given directory. |
| | 210 | + * |
| 202 | 211 | * $params include: |
| 203 | 212 | * dir : storage path directory. |
| 204 | 213 | * |
| — | — | @@ -229,6 +238,7 @@ |
| 230 | 239 | /** |
| 231 | 240 | * Get a local copy on disk of the file at a storage path in the backend. |
| 232 | 241 | * The temporary copy will have the same file extension as the source. |
| | 242 | + * |
| 233 | 243 | * $params include: |
| 234 | 244 | * src : source storage path |
| 235 | 245 | * |
| — | — | @@ -239,9 +249,10 @@ |
| 240 | 250 | |
| 241 | 251 | /** |
| 242 | 252 | * Lock the files at the given storage paths in the backend. |
| 243 | | - * This should either lock all the files or none (on failure). |
| 244 | | - * Do not call this function from places outside FileBackend and FileOp. |
| | 253 | + * This will either lock all the files or none (on failure). |
| 245 | 254 | * |
| | 255 | + * Avoid using this function outside of FileBackendScopedLock. |
| | 256 | + * |
| 246 | 257 | * @param $sources Array Source storage paths |
| 247 | 258 | * @return Status |
| 248 | 259 | */ |
| — | — | @@ -251,14 +262,31 @@ |
| 252 | 263 | |
| 253 | 264 | /** |
| 254 | 265 | * Unlock the files at the given storage paths in the backend. |
| 255 | | - * Do not call this function from places outside FileBackend and FileOp. |
| 256 | 266 | * |
| | 267 | + * Avoid using this function outside of FileBackendScopedLock. |
| | 268 | + * |
| 257 | 269 | * @param $sources Array Source storage paths |
| 258 | 270 | * @return Status |
| 259 | 271 | */ |
| 260 | 272 | final public function unlockFiles( array $paths ) { |
| 261 | 273 | return $this->lockManager->unlock( $paths ); |
| 262 | 274 | } |
| | 275 | + |
| | 276 | + /** |
| | 277 | + * Lock the files at the given storage paths in the backend. |
| | 278 | + * This will either lock all the files or none (on failure). |
| | 279 | + * On failure, the status object will be updated with errors. |
| | 280 | + * |
| | 281 | + * Once the return value goes out scope, the locks will be released and |
| | 282 | + * the status updated. Unlock fatals will not change the status "OK" value. |
| | 283 | + * |
| | 284 | + * @param $sources Array Source storage paths |
| | 285 | + * @param $status Status Status to update on lock/unlock |
| | 286 | + * @return FileBackendScopedLock|null Returns null on failure |
| | 287 | + */ |
| | 288 | + final public function getScopedLock( array $paths, Status $status ) { |
| | 289 | + return FileBackendScopedLock::factory( $this, $paths, $status ); |
| | 290 | + } |
| 263 | 291 | } |
| 264 | 292 | |
| 265 | 293 | /** |
| — | — | @@ -360,7 +388,7 @@ |
| 361 | 389 | } |
| 362 | 390 | |
| 363 | 391 | /** |
| 364 | | - * Whether this backend implements move() and is applies to a potential |
| | 392 | + * Whether this backend implements move() and it applies to a potential |
| 365 | 393 | * move from one storage path to another. No backends hits are required. |
| 366 | 394 | * For example, moving objects accross containers may not be supported. |
| 367 | 395 | * Do not call this function from places outside FileBackend and FileOp. |
| — | — | @@ -384,6 +412,10 @@ |
| 385 | 413 | } |
| 386 | 414 | } |
| 387 | 415 | |
| | 416 | + public function getHashType() { |
| | 417 | + return 'internal'; |
| | 418 | + } |
| | 419 | + |
| 388 | 420 | function streamFile( array $params ) { |
| 389 | 421 | $status = Status::newGood(); |
| 390 | 422 | |
| — | — | @@ -466,8 +498,8 @@ |
| 467 | 499 | $filesToLock = array_merge( $filesToLock, $fileOp->storagePathsUsed() ); |
| 468 | 500 | } |
| 469 | 501 | |
| 470 | | - // Try to lock those files... |
| 471 | | - $status->merge( $this->lockFiles( $filesToLock ) ); |
| | 502 | + // Try to lock those files for the scope of this function... |
| | 503 | + $scopedLock = $this->getScopedLock( $filesToLock, $status ); |
| 472 | 504 | if ( !$status->isOK() ) { |
| 473 | 505 | return $status; // abort |
| 474 | 506 | } |
| — | — | @@ -483,7 +515,6 @@ |
| 484 | 516 | ++$status->failCount; |
| 485 | 517 | $status->success[$index] = false; |
| 486 | 518 | } else { |
| 487 | | - $status->merge( $this->unlockFiles( $filesToLock ) ); |
| 488 | 519 | return $status; |
| 489 | 520 | } |
| 490 | 521 | } |
| — | — | @@ -510,7 +541,6 @@ |
| 511 | 542 | } |
| 512 | 543 | $pos--; |
| 513 | 544 | } |
| 514 | | - $status->merge( $this->unlockFiles( $filesToLock ) ); |
| 515 | 545 | return $status; |
| 516 | 546 | } |
| 517 | 547 | } |
| — | — | @@ -532,12 +562,9 @@ |
| 533 | 563 | $status->merge( $subStatus ); |
| 534 | 564 | } |
| 535 | 565 | |
| 536 | | - // Unlock all the files |
| 537 | | - $status->merge( $this->unlockFiles( $filesToLock ) ); |
| | 566 | + // Make sure status is OK, despite any finish() fatals |
| | 567 | + $status->setResult( true, $status->value ); |
| 538 | 568 | |
| 539 | | - // Make sure status is OK, despite any finish() or unlockFiles() fatals |
| 540 | | - $status->setResult( true ); |
| 541 | | - |
| 542 | 569 | return $status; |
| 543 | 570 | } |
| 544 | 571 | |
| — | — | @@ -648,3 +675,57 @@ |
| 649 | 676 | return $relStoragePath; |
| 650 | 677 | } |
| 651 | 678 | } |
| | 679 | + |
| | 680 | +/** |
| | 681 | + * Class to handle scoped locks, which release when the object is destroyed |
| | 682 | + */ |
| | 683 | +class FileBackendScopedLock { |
| | 684 | + /** @var FileBackendBase */ |
| | 685 | + protected $backend; |
| | 686 | + /** @var Status */ |
| | 687 | + protected $status; |
| | 688 | + /** @var Array List of storage paths*/ |
| | 689 | + protected $paths; |
| | 690 | + |
| | 691 | + /** |
| | 692 | + * @param $backend FileBackendBase |
| | 693 | + * @param $paths Array List of storage paths |
| | 694 | + * @param $status Status |
| | 695 | + */ |
| | 696 | + protected function __construct( FileBackendBase $backend, array $paths, Status $status ) { |
| | 697 | + $this->backend = $backend; |
| | 698 | + $this->paths = $paths; |
| | 699 | + $this->status = $status; |
| | 700 | + } |
| | 701 | + |
| | 702 | + protected function __clone() {} |
| | 703 | + |
| | 704 | + /** |
| | 705 | + * Get a status object resulting from an attempt to lock files. |
| | 706 | + * If the attempt is sucessful, the value of the status will be |
| | 707 | + * FileBackendScopedLock object which releases the locks when |
| | 708 | + * it goes out of scope. Otherwise, the value will be null. |
| | 709 | + * |
| | 710 | + * @param $backend FileBackendBase |
| | 711 | + * @param $files Array List of storage paths |
| | 712 | + * @param $status Status |
| | 713 | + * @return FileBackendScopedLock|null |
| | 714 | + */ |
| | 715 | + public static function factory( FileBackendBase $backend, array $files, Status $status ) { |
| | 716 | + $lockStatus = $backend->lockFiles( $files ); |
| | 717 | + $status->merge( $lockStatus ); |
| | 718 | + if ( $lockStatus->isOK() ) { |
| | 719 | + return new self( $backend, $files, $status ); |
| | 720 | + } |
| | 721 | + return null; |
| | 722 | + } |
| | 723 | + |
| | 724 | + function __destruct() { |
| | 725 | + $wasOk = $this->status->isOK(); |
| | 726 | + $this->status->merge( $this->backend->unlockFiles( $this->paths ) ); |
| | 727 | + if ( $wasOk ) { |
| | 728 | + // Make sure status is OK, despite any unlockFiles() fatals |
| | 729 | + $this->status->setResult( true, $this->status->value ); |
| | 730 | + } |
| | 731 | + } |
| | 732 | +} |