Merge pull request #1038 from krlmlr/b-clear-before-finalize

Clear external pointer before calling finalizer

Author: eddelbuettel

inst/include/Rcpp/XPtr.h

@@ -33,11 +33,19 @@ void standard_delete_finalizer(T* obj) {
 
 template <typename T, void Finalizer(T*) >
 void finalizer_wrapper(SEXP p) {
-    if (TYPEOF(p) == EXTPTRSXP) {
-        T* ptr = (T*) R_ExternalPtrAddr(p);
-        RCPP_DEBUG_3("finalizer_wrapper<%s>(SEXP p = <%p>). ptr = %p", DEMANGLE(T), p, ptr)
-        Finalizer(ptr);
-    }
+    if (TYPEOF(p) != EXTPTRSXP)
+        return;
+
+    T* ptr = (T*) R_ExternalPtrAddr(p);
+    RCPP_DEBUG_3("finalizer_wrapper<%s>(SEXP p = <%p>). ptr = %p", DEMANGLE(T), p, ptr)
+
+    if (ptr == NULL)
+        return;
+
+    // Clear before finalizing to avoid behavior like access of freed memory
+    R_ClearExternalPtr(p);
+
+    Finalizer(ptr);
 }
 
 template <
@@ -174,10 +182,9 @@ class XPtr :
             // Call the finalizer -- note that this implies that finalizers
             // need to be ready for a NULL external pointer value (our
             // default C++ finalizer is since delete NULL is a no-op).
+            // This clears the external pointer just before calling the finalizer,
+            // to avoid interesting behavior with co-dependent finalizers.
             finalizer_wrapper<T,Finalizer>(Storage::get__());
-
-            // Clear the external pointer
-            R_ClearExternalPtr(Storage::get__());
         }
     }