Skip to content

Commit

Permalink
TempFSFile: Use a WeakMap for reference tracking if available
Browse files Browse the repository at this point in the history
Use a WeakMap to track references to TempFSFile instances if available
to avoid deprecation warnings for dynamic property creation on PHP 8.2.
Use an anonymous class rather than an stdClass to test the behavior in
TempFSFileTestTrait to verify that bind() does not trigger dynamic
property creation on PHP 8.2.

Also add a Phan stub for WeakMap for PHP 7.4.

Bug: T324894
Change-Id: Ic413c115e9ed1c750e175152094f3309628e777a
  • Loading branch information
mszabo-wikia committed Feb 12, 2023
1 parent c08c6ce commit 4e3c6cb
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 5 deletions.
1 change: 1 addition & 0 deletions .phan/config.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
class_exists( Socket::class ) ? [] : [ '.phan/stubs/Socket.php' ],
class_exists( ReturnTypeWillChange::class ) ? [] : [ '.phan/stubs/ReturnTypeWillChange.php' ],
class_exists( AllowDynamicProperties::class ) ? [] : [ '.phan/stubs/AllowDynamicProperties.php' ],
class_exists( WeakMap::class ) ? [] : [ '.phan/stubs/WeakMap.php' ],
[
// This makes constants and globals known to Phan before processing all other files.
// You can check the parser order with --dump-parsed-file-list
Expand Down
24 changes: 24 additions & 0 deletions .phan/stubs/WeakMap.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

// Phan stub for PHP 8.0's WeakMap class

final class WeakMap implements ArrayAccess, Countable, IteratorAggregate {

public function count(): int {
}

public function getIterator(): Iterator {
}

public function offsetExists( object $object ): bool {
}

public function offsetGet( object $object ): mixed {
}

public function offsetSet( object $object, mixed $value ): void {
}

public function offsetUnset( object $object ): void {
}
}
27 changes: 23 additions & 4 deletions includes/libs/filebackend/fsfile/TempFSFile.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ class TempFSFile extends FSFile {
/** @var array Map of (path => 1) for paths to delete on shutdown */
protected static $pathsCollect = null;

/**
* Map objects referencing this temporary file to this instance.
* This is used to delay garbage collecting (and thereby removing) this temporary file
* until all references to it have been destroyed.
* @var WeakMap|null
*/
private $references;

/**
* Do not call directly. Use TempFSFileFactory
*
Expand All @@ -52,6 +60,11 @@ public function __construct( $path ) {
register_shutdown_function( [ __CLASS__, 'purgeAllOnShutdown' ] );
// @codeCoverageIgnoreEnd
}

// Use a WeakMap on PHP >= 8.0 for reference tracking to avoid dynamic property creation (T324894)
if ( class_exists( WeakMap::class ) ) {
$this->references = new WeakMap();
}
}

/**
Expand Down Expand Up @@ -130,11 +143,17 @@ public function purge() {
*/
public function bind( $object ) {
if ( is_object( $object ) ) {
if ( !isset( $object->tempFSFileReferences ) ) {
// Init first since $object might use __get() and return only a copy variable
$object->tempFSFileReferences = [];
// Use a WeakMap on PHP >= 8.0 to avoid dynamic property creation (T324894)
if ( $this->references !== null ) {
$this->references[$object] = $this;
} else {
// PHP 7.4
if ( !isset( $object->tempFSFileReferences ) ) {
// Init first since $object might use __get() and return only a copy variable
$object->tempFSFileReferences = [];
}
$object->tempFSFileReferences[] = $this;
}
$object->tempFSFileReferences[] = $this;
}

return $this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ public function testBind() {
$file = $this->newFile();
$path = $file->getPath();
$this->assertTrue( is_file( $path ) );
$obj = (object)[];
$obj = new class {
};
$file->bind( $obj );
unset( $file );
$this->assertTrue( is_file( $path ) );
Expand Down

0 comments on commit 4e3c6cb

Please sign in to comment.