rdbms: Remove instance ownership concept

Instance ownership is supposed to protect LoadBalancer and Database
against unauthorized calls to internal methods other than by the owning
LoadBalancer/LBFactory. This seems like unnecessary complexity. It was
introduced for T231443 and T217819, but the link was speculative and in
the end it didn't help to fix or isolate those bugs. Since then it has
caused a production error (T303885) and an intermittent CI failure
(T292239).

Instead, split the ILoadBalancer interface, introducing
ILoadBalancerForOwner, which contains the methods which are only safe
to call by LBFactory or by the caller of LBFactory::newMainLB(). This
allows phan to statically detect inappropriate calls to internal
methods.

Ownership was used for convenience for two things unrelated to its
original purpose:

* Suppressing calls to ScopedCallback::newScopedIgnoreUserAbort() when
  the caller has already called it. But nested calls are apparently
  harmless, so I just called it unconditionally.
* Suppressing exceptions from Database::close(). I extended the
  behaviour for owned instances to apply to all instances, so even
  unowned instances will no longer throw on close.

CodeSearch suggests nothing in extensions is calling these methods with
an owner parameter. One extension (Wikibase) overrides a method with an
owner parameter in a test mock class and so needs to be simultaneously
updated.

Depends-On: Ib03aba9d8f5f05b875a321d00b14483633a636a8
Change-Id: I27ba4973d24d759c88b3868c95e7db875801ca0c

1 files changed, 15 insertions, 41 deletions

diff --git a/tests/phpunit/unit/includes/libs/rdbms/database/DatabaseSQLTest.php b/tests/phpunit/unit/includes/libs/rdbms/database/DatabaseSQLTest.php
index 188661983fbb..da4450d33169 100644
--- a/tests/phpunit/unit/includes/libs/rdbms/database/DatabaseSQLTest.php
+++ b/tests/phpunit/unit/includes/libs/rdbms/database/DatabaseSQLTest.php
@@ -2441,15 +2441,7 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
 			$this->database->query( 'SELECT 2', $fname );
 		} );
 		$this->database->delete( 'x', [ 'field' => 3 ], __METHOD__ );
-		try {
-			$this->database->close();
-			$this->fail( 'Expected exception not thrown' );
-		} catch ( DBUnexpectedError $ex ) {
-			$this->assertSame(
-				"Wikimedia\Rdbms\Database::close: transaction is still open (from $fname)",
-				$ex->getMessage()
-			);
-		}
+		$this->database->close();
 
 		$this->assertFalse( $this->database->isOpen() );
 		$this->assertLastSql( 'BEGIN; DELETE FROM x WHERE field = 3; ROLLBACK; SELECT 2' );
@@ -2460,25 +2452,16 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
 	 * @covers \Wikimedia\Rdbms\Database::close
 	 */
 	public function testPrematureClose2() {
-		try {
-			$fname = __METHOD__;
-			$this->database->startAtomic( __METHOD__ );
-			$this->database->onTransactionCommitOrIdle( function () use ( $fname ) {
-				$this->database->query( 'SELECT 1', $fname );
-			} );
-			$this->database->onAtomicSectionCancel( function () use ( $fname ) {
-				$this->database->query( 'SELECT 2', $fname );
-			} );
-			$this->database->delete( 'x', [ 'field' => 3 ], __METHOD__ );
-			$this->database->close();
-			$this->fail( 'Expected exception not thrown' );
-		} catch ( DBUnexpectedError $ex ) {
-			$this->assertSame(
-				'Wikimedia\Rdbms\Database::close: atomic sections ' .
-				'DatabaseSQLTest::testPrematureClose2 are still open',
-				$ex->getMessage()
-			);
-		}
+		$fname = __METHOD__;
+		$this->database->startAtomic( __METHOD__ );
+		$this->database->onTransactionCommitOrIdle( function () use ( $fname ) {
+			$this->database->query( 'SELECT 1', $fname );
+		} );
+		$this->database->onAtomicSectionCancel( function () use ( $fname ) {
+			$this->database->query( 'SELECT 2', $fname );
+		} );
+		$this->database->delete( 'x', [ 'field' => 3 ], __METHOD__ );
+		$this->database->close();
 
 		$this->assertFalse( $this->database->isOpen() );
 		$this->assertLastSql( 'BEGIN; DELETE FROM x WHERE field = 3; ROLLBACK; SELECT 2' );
@@ -2489,19 +2472,10 @@ class DatabaseSQLTest extends PHPUnit\Framework\TestCase {
 	 * @covers \Wikimedia\Rdbms\Database::close
 	 */
 	public function testPrematureClose3() {
-		try {
-			$this->database->setFlag( IDatabase::DBO_TRX );
-			$this->database->delete( 'x', [ 'field' => 3 ], __METHOD__ );
-			$this->assertSame( 1, $this->database->trxLevel() );
-			$this->database->close();
-			$this->fail( 'Expected exception not thrown' );
-		} catch ( DBUnexpectedError $ex ) {
-			$this->assertSame(
-				'Wikimedia\Rdbms\Database::close: ' .
-				'expected mass rollback of all peer transactions (DBO_TRX set)',
-				$ex->getMessage()
-			);
-		}
+		$this->database->setFlag( IDatabase::DBO_TRX );
+		$this->database->delete( 'x', [ 'field' => 3 ], __METHOD__ );
+		$this->assertSame( 1, $this->database->trxLevel() );
+		$this->database->close();
 
 		$this->assertFalse( $this->database->isOpen() );
 		$this->assertLastSql( 'BEGIN; DELETE FROM x WHERE field = 3; ROLLBACK' );