be more careful about potential gc of pyptrs

src/PyCall.jl

@@ -79,7 +79,7 @@ PyPtr(o::PyObject) = getfield(o, :o)
 """
     ≛(x, y)
 
-`PyPtr` based comparison of `x` and `y`, which can be of type `PyObject` or `PyPtr`. 
+`PyPtr` based comparison of `x` and `y`, which can be of type `PyObject` or `PyPtr`.
 """
 ≛(o1::Union{PyObject,PyPtr}, o2::Union{PyObject,PyPtr}) = PyPtr(o1) == PyPtr(o2)
 
@@ -150,12 +150,12 @@ a `PyObject`, the refcount is incremented.  Otherwise a `PyObject`
 wrapping/converted from `x` is created.
 """
 pyreturn(x::Any) = pystealref!(PyObject(x))
-pyreturn(x::PyObject) = pyincref_(PyPtr(x))
+pyreturn(o::PyObject) = PyPtr(pyincref(o))
 
 function Base.copy!(dest::PyObject, src::PyObject)
     pydecref(dest)
-    setfield!(dest, :o, PyPtr(src))
-    return pyincref(dest)
+    setfield!(dest, :o, PyPtr(pyincref(src)))
+    return dest
 end
 
 pyisinstance(o::PyObject, t::PyObject) =
@@ -273,7 +273,7 @@ function hash(o::PyObject)
     elseif is_pyjlwrap(o)
         # call native Julia hash directly on wrapped Julia objects,
         # since on 64-bit Windows the Python 2.x hash is only 32 bits
-        hashsalt(unsafe_pyjlwrap_to_objref(PyPtr(o)))
+        hashsalt(unsafe_pyjlwrap_to_objref(o))
     else
         h = ccall((@pysym :PyObject_Hash), Py_hash_t, (PyPtr,), o)
         if h == -1 # error
@@ -303,7 +303,7 @@ end
 
 getproperty(o::PyObject, s::Symbol) = convert(PyAny, getproperty(o, string(s)))
 
-propertynames(o::PyObject) = map(x->Symbol(first(x)), 
+propertynames(o::PyObject) = map(x->Symbol(first(x)),
                                 pycall(inspect."getmembers", PyObject, o))
 
 # avoiding method ambiguity

src/pydates.jl

@@ -133,18 +133,19 @@ end
 
 function convert(::Type{Dates.DateTime}, o::PyObject)
     if PyDate_Check(o)
-        dt = convert(Ptr{UInt8}, PyPtr(o)) + PyDate_HEAD
-        if PyDateTime_Check(o)
-            Dates.DateTime((UInt(unsafe_load(dt,1))<<8)|unsafe_load(dt,2), # Y
-                           unsafe_load(dt,3), unsafe_load(dt,4), # month, day
-                           unsafe_load(dt,5), unsafe_load(dt,6), # hour, minute
-                           unsafe_load(dt,7), # second
-                           div((UInt(unsafe_load(dt,8)) << 16) |
-                               (UInt(unsafe_load(dt,9)) << 8) |
-                               unsafe_load(dt,10), 1000)) # μs ÷ 1000
-        else
-            Dates.DateTime((UInt(unsafe_load(dt,1))<<8)|unsafe_load(dt,2), # Y
-                           unsafe_load(dt,3), unsafe_load(dt,4)) # month, day
+        GC.@preserve o let dt = convert(Ptr{UInt8}, PyPtr(o)) + PyDate_HEAD
+            if PyDateTime_Check(o)
+                Dates.DateTime((UInt(unsafe_load(dt,1))<<8)|unsafe_load(dt,2), # Y
+                            unsafe_load(dt,3), unsafe_load(dt,4), # month, day
+                            unsafe_load(dt,5), unsafe_load(dt,6), # hour, minute
+                            unsafe_load(dt,7), # second
+                            div((UInt(unsafe_load(dt,8)) << 16) |
+                                (UInt(unsafe_load(dt,9)) << 8) |
+                                unsafe_load(dt,10), 1000)) # μs ÷ 1000
+            else
+                Dates.DateTime((UInt(unsafe_load(dt,1))<<8)|unsafe_load(dt,2), # Y
+                            unsafe_load(dt,3), unsafe_load(dt,4)) # month, day
+            end
         end
     else
         throw(ArgumentError("unknown DateTime type $o"))
@@ -153,17 +154,18 @@ end
 
 function convert(::Type{Dates.Date}, o::PyObject)
     if PyDate_Check(o)
-        dt = convert(Ptr{UInt8}, PyPtr(o)) + PyDate_HEAD
-        Dates.Date((UInt(unsafe_load(dt,1)) << 8) | unsafe_load(dt,2), # Y
-                   unsafe_load(dt,3), unsafe_load(dt,4)) # month, day
+        GC.@preserve o let dt = convert(Ptr{UInt8}, PyPtr(o)) + PyDate_HEAD
+            Dates.Date((UInt(unsafe_load(dt,1)) << 8) | unsafe_load(dt,2), # Y
+                    unsafe_load(dt,3), unsafe_load(dt,4)) # month, day
+        end
     else
         throw(ArgumentError("unknown Date type $o"))
     end
 end
 
 function delta_dsμ(o::PyObject)
     PyDelta_Check(o) || throw(ArgumentError("$o is not a timedelta instance"))
-    p = unsafe_load(convert(Ptr{PyDateTime_Delta{Py_hash_t}}, PyPtr(o)))
+    p = GC.@preserve o unsafe_load(convert(Ptr{PyDateTime_Delta{Py_hash_t}}, PyPtr(o)))
     return (p.days, p.seconds, p.microseconds)
 end
 

src/pyoperators.jl

@@ -59,8 +59,7 @@ for (op,py) in ((:<, Py_LT), (:<=, Py_LE), (:(==), Py_EQ), (:!=, Py_NE),
         if ispynull(o1) || ispynull(o2)
             return $(py==Py_EQ || py==Py_NE || op==:isless ? :($op(PyPtr(o1), PyPtr(o2))) : false)
         elseif is_pyjlwrap(o1) && is_pyjlwrap(o2)
-            return $op(unsafe_pyjlwrap_to_objref(PyPtr(o1)),
-                       unsafe_pyjlwrap_to_objref(PyPtr(o2)))
+            return $op(unsafe_pyjlwrap_to_objref(o1), unsafe_pyjlwrap_to_objref(o2))
         else
             if $(op == :isless || op == :isequal)
                 return Bool(@pycheckz ccall((@pysym :PyObject_RichCompareBool), Cint,
@@ -79,5 +78,5 @@ for (op,py) in ((:<, Py_LT), (:<=, Py_LE), (:(==), Py_EQ), (:!=, Py_NE),
     end
 end
 # default to false since hash(x) != hash(PyObject(x)) in general
-isequal(o1::PyObject, o2::Any) = !ispynull(o1) && is_pyjlwrap(o1) ? isequal(unsafe_pyjlwrap_to_objref(PyPtr(o1)), o2) : false
+isequal(o1::PyObject, o2::Any) = !ispynull(o1) && is_pyjlwrap(o1) ? isequal(unsafe_pyjlwrap_to_objref(o1), o2) : false
 isequal(o1::Any, o2::PyObject) = isequal(o2, o1)

src/pytype.jl

@@ -341,7 +341,7 @@ function pyjlwrap_dealloc(o::PyPtr)
 end
 
 unsafe_pyjlwrap_to_objref(o::Union{PyPtr, PyObject}) =
-  unsafe_pointer_to_objref(unsafe_load(convert(Ptr{Ptr{Cvoid}}, PyPtr(o)), 4))
+  GC.@preserve o unsafe_pointer_to_objref(unsafe_load(convert(Ptr{Ptr{Cvoid}}, PyPtr(o)), 4))
 
 function pyjlwrap_repr(o::PyPtr)
     try
@@ -425,9 +425,6 @@ pyjlwrap_type(init::Function, name::AbstractString) =
     pyjlwrap_type!(init, PyTypeObject(), name)
 
 # Given a jlwrap type, create a new instance (and save value for gc)
-# (Careful: not sure if this works if value is an isbits type,
-#  since, the jl_value_t* may be to a temporary copy.  But don't need
-#  to wrap isbits types in Python objects anyway.)
 function pyjlwrap_new(pyT::PyTypeObject, value::Any)
     # TODO change to `Ref{PyTypeObject}` when 0.6 is dropped.
     o = PyObject(@pycheckn ccall((@pysym :_PyObject_New),

src/serialize.jl

@@ -9,7 +9,7 @@ pickle() = ispynull(_pickle) ? copy!(_pickle, pyimport(PyCall.pyversion.major 
 function serialize(s::AbstractSerializer, pyo::PyObject)
     serialize_type(s, PyObject)
     if ispynull(pyo)
-        serialize(s, PyPtr(pyo))
+        serialize(s, PyPtr_NULL)
     else
         b = PyBuffer(pycall(pickle()."dumps", PyObject, pyo))
         serialize(s, unsafe_wrap(Array, Ptr{UInt8}(pointer(b)), sizeof(b)))

test/runtests.jl

@@ -461,7 +461,7 @@ const PyInt = pyversion < v"3" ? Int : Clonglong
     let o = PyObject(Any[1,2]), c = o
         broadcast!(+, o, o, Any[3,4]) # o .+= x doesn't work yet in 0.7
         @test collect(o) == [1,2,3,4]
-        @test PyPtr(o) == PyPtr(c) # updated in-place
+        GC.@preserve o c @test(PyPtr(o) == PyPtr(c)) # updated in-place
     end
 
     # more flexible bool conversions, matching Python "truth value testing"