[PHP] Fix cleanup code handling issues

Fix to call cleanup code in exception situations and not to invoke
the freearg typemap twice in certain situations.

Fixes https://sourceforge.net/p/swig/bugs/1211/

CHANGES.current

@@ -7,6 +7,11 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/
 Version 4.1.0 (in progress)
 ===========================
 
+2022-02-17: olly
+	    [PHP] https://sourceforge.net/p/swig/bugs/1211/
+	    Fix to call cleanup code in exception situations and not to invoke
+	    the freearg typemap twice in certain situations.
+
 2022-02-15: olly
 	    #300 #368 Improve parser handling of % followed immediately by
 	    an identifier.  If it's not a recognised directive the scanner

Examples/test-suite/common.mk

@@ -234,6 +234,7 @@ CPP_TEST_CASES += \
 	evil_diamond_ns \
 	evil_diamond_prop \
 	exception_classname \
+	exception_memory_leak \
 	exception_order \
 	extend \
 	extend_constructor_destructor \

Examples/test-suite/exception_memory_leak.i

@@ -1,4 +1,4 @@
-%module r_memory_leak
+%module exception_memory_leak
 
 %include <std_string.i>
 
@@ -8,33 +8,38 @@
 }
 %typemap(freearg) Foo* foo
 {
-  printf("    \"        Object deleted\"\n");
+  Foo::inc_freearg_count();
   delete $1;
 }
 %typemap(out) Foo* verify_no_memory_leak
 {
   if ($1 == NULL)
     SWIG_exception_fail(SWIG_RuntimeError, "Let's see how the bindings manage this exception!");
+  $1 = NULL;
 }
-%typemap(scoerceout) Foo*
-  %{ if (!is.null($result) && !is.logical($result)) {$result <- new("$R_class", ref=$result) ;}; %}
 
 %inline %{
   #include <string>
 
   class Foo {
       static unsigned count;
+      static unsigned freearg_count;
     public:
       Foo() { ++count; }
       ~Foo() { --count; }
       static unsigned get_count() { return count; }
+      static unsigned get_freearg_count() { return freearg_count; }
+#ifndef SWIG
+      static void inc_freearg_count() { ++freearg_count; }
+#endif
   };
 
   unsigned Foo::count = 0;
+  unsigned Foo::freearg_count = 0;
 
   static Foo* trigger_internal_swig_exception(const std::string& message, Foo* foo)
   {
     return (message == "null") ? NULL : foo;
-  };
+  }
 
 %}

Examples/test-suite/php/exception_memory_leak_runme.php

@@ -0,0 +1,23 @@
+<?php
+
+require "tests.php";
+
+check::functions(array('trigger_internal_swig_exception'));
+check::classes(array('Foo', 'exception_memory_leak'));
+// No new vars
+check::globals(array());
+
+$a = new Foo();
+check::equal(Foo::get_count(), 1, "Should have 1 Foo objects");
+$b = new Foo();
+check::equal(Foo::get_count(), 2, "Should have 2 Foo objects");
+
+// Normal behaviour
+trigger_internal_swig_exception("no problem", $a);
+check::equal(Foo::get_count(), 2, "Should have 2 Foo objects");
+check::equal(Foo::get_freearg_count(), 1, "freearg should have been used once");
+
+// SWIG exception triggered and handled.
+trigger_internal_swig_exception("null", $b);
+check::equal(Foo::get_count(), 2, "Should have 2 Foo objects");
+check::equal(Foo::get_freearg_count(), 2, "freearg should have been used twice");

Examples/test-suite/r/Makefile.in

@@ -18,7 +18,6 @@ C_TEST_CASES += \
 
 CPP_TEST_CASES += \
 	r_double_delete \
-	r_memory_leak \
 	r_overload_array \
 	r_sexp \
         r_overload_comma \

Examples/test-suite/r/exception_memory_leak_runme.R

@@ -1,8 +1,8 @@
 clargs <- commandArgs(trailing=TRUE)
 source(file.path(clargs[1], "unittest.R"))
 
-dyn.load(paste("r_memory_leak", .Platform$dynlib.ext, sep=""))
+dyn.load(paste("exception_memory_leak", .Platform$dynlib.ext, sep=""))
-source("r_memory_leak.R")
+source("exception_memory_leak.R")
 cacheMetaData(1)
 
 a <- Foo();
@@ -13,6 +13,7 @@ unittest(Foo_get_count(), 2);
 # Normal behaviour
 invisible(trigger_internal_swig_exception("no problem", a));
 unittest(Foo_get_count(), 2);
+unittest(Foo_get_freearg_count(), 1);
 # SWIG exception introduced
 result <- tryCatch({
   trigger_internal_swig_exception("null", b);
@@ -24,3 +25,4 @@ result <- tryCatch({
   unittest(1,1);
 })
 unittest(Foo_get_count(), 2);
+unittest(Foo_get_freearg_count(), 2);

Lib/php/php.swg

@@ -191,7 +191,7 @@
     if (!(Z_ISREF($input) && Z_ISNULL_P(Z_REFVAL($input)))) {
       /* wasn't a pre/ref/thing, OR anything like an int thing */
       zend_throw_exception(zend_ce_type_error, "Type error in argument $arg of $symname", 0);
-      return;
+      goto fail;
     }
   }
   force=0;
@@ -555,18 +555,18 @@
                  unsigned long,
                  unsigned short %{
   zend_throw_exception(NULL, "C++ $1_type exception thrown", $1);
-  return;
+  goto fail;
 %}
 
 %typemap(throws) SWIGTYPE, SWIGTYPE &, SWIGTYPE &&, SWIGTYPE *, SWIGTYPE [], SWIGTYPE [ANY] %{
   (void)$1;
   zend_throw_exception(NULL, "C++ $1_type exception thrown", 0);
-  return;
+  goto fail;
 %}
 
 %typemap(throws) char * %{
   zend_throw_exception(NULL, $1, 0);
-  return;
+  goto fail;
 %}
 
 /* Array reference typemaps */

Lib/php/std_string.i

@@ -51,7 +51,7 @@ namespace std {
 
     %typemap(throws) string, const string& %{
         zend_throw_exception(NULL, $1.c_str(), 0);
-        return;
+        goto fail;
     %}
 
     %typemap(in, phptype="string") const string & ($*1_ltype temp) %{

Source/Modules/php.cxx

@@ -1031,6 +1031,7 @@ public:
     Printf(f->code, "\tWRONG_PARAM_COUNT;\n}\n\n");
     Printf(f->code, "  if(!arg) {\n");
     Printf(f->code, "    zend_throw_exception(zend_ce_type_error, \"this pointer is NULL\", 0);\n");
+    Printf(f->code, "    return;\n");
     Printf(f->code, "  }\n");
     Printf(f->code, "  arg2 = Z_STR(args[0]);\n\n");
 
@@ -1405,7 +1406,7 @@ public:
       Printv(f->code, outarg, NIL);
     }
 
-    if (cleanup) {
+    if (static_setter && cleanup) {
       Printv(f->code, cleanup, NIL);
     }
 
@@ -1698,8 +1699,10 @@ public:
 		 "#ifndef SWIG_PHP_INTERFACE_", interface, "_CE\n",
 		 "  {\n",
 		 "    zend_class_entry *swig_interface_ce = zend_lookup_class(zend_string_init(\"", interface, "\", sizeof(\"", interface, "\") - 1, 0));\n",
-		 "    if (!swig_interface_ce) zend_throw_exception(zend_ce_error, \"Interface \\\"", interface, "\\\" not found\", 0);\n",
+		 "    if (swig_interface_ce)\n",
-		 "    zend_do_implement_interface(SWIG_Php_ce_", class_name, ", swig_interface_ce);\n",
+		 "      zend_do_implement_interface(SWIG_Php_ce_", class_name, ", swig_interface_ce);\n",
+                 "    else\n",
+                 "      zend_throw_exception(zend_ce_error, \"Interface \\\"", interface, "\\\" not found\", 0);\n",
 		 "  }\n",
 		 "#endif\n",
 		 NIL);