MDL-49998 unit tests: Close the pdf objects when needed.
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Sun, 26 Apr 2015 22:59:39 +0000 (00:59 +0200)
committerEloy Lafuente (stronk7) <stronk7@moodle.org>
Tue, 28 Apr 2015 17:46:50 +0000 (19:46 +0200)
When a pdf object is instantiated and any file is loaded
with set_pdf() or load_pdf(), the files remain open until
the pdf is saved with pdf_save() or outputted with Output()..

In that cases is needed to perform an explicit Close() in
order to free resources, parsers and, ultimately, fclose()
the files.

Note that only the uses detected in the editpdf unit tests have
been fixed. I'd recommend to analyse every instance of the tcpdf
libs.

Also, there was one incorrecly reused pdf instance in then
generate_combined_pdf_for_attempt() method. Apparently it was not
leading to problems, but better use a separate instance (#246).

mod/assign/feedback/editpdf/classes/document_services.php
mod/assign/feedback/editpdf/classes/pdf.php

index 5c60de9..9f09c29 100644 (file)
@@ -243,15 +243,16 @@ class document_services {
 
         // Detect corrupt generated pdfs and replace with a blank one.
         if ($files) {
+            $pdf = new pdf();
             $pagecount = $pdf->load_pdf($tmpfile);
             if ($pagecount <= 0) {
                 $files = false;
             }
+            $pdf->Close(); // PDF loaded and never saved/outputted needs to be closed.
         }
 
         if (!$files) {
             // This was a blank pdf.
-            unset($pdf);
             $pdf = new pdf();
             $content = $pdf->Output(self::COMBINED_PDF_FILENAME, 'S');
             $file = $fs->create_file_from_string($record, $content);
@@ -314,6 +315,7 @@ class document_services {
         // Get the total number of pages.
         $pdf = new pdf();
         $pagecount = $pdf->set_pdf($combined);
+        $pdf->Close(); // PDF loaded and never saved/outputted needs to be closed.
 
         // Delete temporary folders and files.
         @unlink($combined);
@@ -375,6 +377,7 @@ class document_services {
             $files[$i] = $fs->create_file_from_pathname($record, $tmpdir . '/' . $image);
             @unlink($tmpdir . '/' . $image);
         }
+        $pdf->Close(); // PDF loaded and never saved/outputted needs to be closed.
 
         @unlink($combined);
         @rmdir($tmpdir);
index e2884d8..8578bd6 100644 (file)
@@ -485,13 +485,13 @@ class pdf extends \FPDI {
             // PDF was not valid - try running it through ghostscript to clean it up.
             $pagecount = 0;
         }
+        $pdf->Close(); // PDF loaded and never saved/outputted needs to be closed.
 
         if ($pagecount > 0) {
             // Page is valid and can be read by tcpdf.
             return $tempsrc;
         }
 
-
         $gsexec = \escapeshellarg($CFG->pathtogs);
         $tempdstarg = \escapeshellarg($tempdst);
         $tempsrcarg = \escapeshellarg($tempsrc);
@@ -511,6 +511,8 @@ class pdf extends \FPDI {
             // PDF was not valid - try running it through ghostscript to clean it up.
             $pagecount = 0;
         }
+        $pdf->Close(); // PDF loaded and never saved/outputted needs to be closed.
+
         if ($pagecount <= 0) {
             @unlink($tempdst);
             // Could not parse the converted pdf.
@@ -572,6 +574,7 @@ class pdf extends \FPDI {
             $ret->status = self::GSPATH_ERROR;
             $ret->message = $e->getMessage();
         }
+        $pdf->Close(); // PDF loaded and never saved/outputted needs to be closed.
 
         return $ret;
     }