From 97e4e113e44a6be48ca9e8882e1a4dd5f8eefd7b Mon Sep 17 00:00:00 2001
From: Brian Wolff <bawolff+wn@gmail.com>
Date: Mon, 25 May 2015 15:21:59 -0600
Subject: [PATCH] Move xml_free_parser out of destructor.

Attempt to reduce error log spam. I'm not sure about the exact
cause of this error, nor have I really reproduced locally, so
I don't know if this will help, but getting it out of destructor
seems like a good idea.

I also do not know if calling xml_free_parser is really needed
(Would php automatically destroy parser during garbage collection?)

Bug: T89532
Change-Id: I5709b6cf26cf4a9032a79d2b5f48ed3b4238eaee
---
 includes/media/XMP.php | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/includes/media/XMP.php b/includes/media/XMP.php
index 042f7490ea..6b36e37d3f 100644
--- a/includes/media/XMP.php
+++ b/includes/media/XMP.php
@@ -155,16 +155,27 @@ class XMPReader implements LoggerAwareInterface {
 		$this->logger = $logger;
 	}
 
+	/**
+	 * free the XML parser.
+	 *
+	 * @note It is unclear to me if we really need to do this ourselves
+	 *  or if php garbage collection will automatically free the xmlParser
+	 *  when it is no longer needed.
+	 */
+	private function destroyXMLParser() {
+		if ( $this->xmlParser ) {
+			xml_parser_free( $this->xmlParser );
+			$this->xmlParser = null;
+		}
+	}
+
 	/**
 	 * Main use is if a single item has multiple xmp documents describing it.
 	 * For example in jpeg's with extendedXMP
 	 */
 	private function resetXMLParser() {
 
-		if ( $this->xmlParser ) {
-			//is this needed?
-			xml_parser_free( $this->xmlParser );
-		}
+		$this->destroyXMLParser();
 
 		$this->xmlParser = xml_parser_create_ns( 'UTF-8', ' ' );
 		xml_parser_set_option( $this->xmlParser, XML_OPTION_CASE_FOLDING, 0 );
@@ -180,15 +191,6 @@ class XMPReader implements LoggerAwareInterface {
 		$this->xmlParsableBuffer = '';
 	}
 
-	/** Destroy the xml parser
-	 *
-	 * Not sure if this is actually needed.
-	 */
-	function __destruct() {
-		// not sure if this is needed.
-		xml_parser_free( $this->xmlParser );
-	}
-
 	/**
 	 * Check if this instance supports using this class
 	 */
@@ -294,12 +296,11 @@ class XMPReader implements LoggerAwareInterface {
 	 *
 	 * @param string $content XMP data
 	 * @param bool $allOfIt If this is all the data (true) or if its split up (false). Default true
-	 * @param bool $reset Does xml parser need to be reset. Default false
 	 * @throws RuntimeException
 	 * @return bool Success.
 	 */
-	public function parse( $content, $allOfIt = true, $reset = false ) {
-		if ( $reset ) {
+	public function parse( $content, $allOfIt = true ) {
+		if ( !$this->xmlParser ) {
 			$this->resetXMLParser();
 		}
 		try {
@@ -373,14 +374,21 @@ class XMPReader implements LoggerAwareInterface {
 
 				$this->logger->info( "XMPReader::parse : Error reading XMP content: $error ($where)" );
 				$this->results = array(); // blank if error.
+				$this->destroyXMLParser();
 				return false;
 			}
 		} catch ( Exception $e ) {
 			$this->logger->info( 'XMP parse error: ' . $e );
 			$this->results = array();
 
+			if ( $allOfIt ) {
+				$this->destroyXMLParser();
+			}
 			return false;
 		}
+		if ( $allOfIt ) {
+			$this->destroyXMLParser();
+		}
 
 		return true;
 	}
-- 
2.20.1