Skip to content

Add PyHandle #1087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Add PyHandle #1087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Add PyHandle
  • Loading branch information
@amos402
amos402 committed Mar 10, 2020
commit e804d772101870b82e421918df607aa5b75d4674
1 change: 1 addition & 0 deletions src/runtime/Python.Runtime.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@
<Compile Include="pyansistring.cs" />
<Compile Include="pydict.cs" />
<Compile Include="pyfloat.cs" />
<Compile Include="pyhandle.cs" />
<Compile Include="pyint.cs" />
<Compile Include="pyiter.cs" />
<Compile Include="pylist.cs" />
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/exceptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ public static void SetError(Exception e)
/// </remarks>
public static bool ErrorOccurred()
{
return Runtime.PyErr_Occurred() != IntPtr.Zero;
return Runtime.PyErr_Occurred() != PyHandle.Null;
}

/// <summary>
Expand Down
172 changes: 172 additions & 0 deletions src/runtime/pyhandle.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
using System;
using System.Runtime.InteropServices;

namespace Python.Runtime
{
struct PyHandle : IDisposable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having regular structs IDisposable is a bad idea. The following two cases would cause the pointer to be copied and potentially lead to double free:

var x = GetNewPyHandleToSomething();
var y = x;
x.Dispose();
y.Dispose();
var x = GetNewPyHandleToSomething();
void DoSomethingWithHandle(PyHandle handle) {
  ...
  handle.Dispose();
}
DoSomethingWithHandle(x);
x.Dispose();

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This merely for easy using statement like using(var list = PyList_New()), people should know what they're doing when they using the C API, if they're not, everything on language sight are just smoke and mirrors. Just like you can always decref a wrong object.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amos402 but they only have to use raw C API if you expose it. In that sense *Reference types are fool-proof, unless you use DagerousGetAddress.

C-style API is a error-prone mental burden, which is totally unnecessary in this case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not agree with it, since you can't control the user how write the code, these fool-proof are helpless.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amos402 users of Python.NET do not need to work with pointers at all.

In PRs for Python.NET I can safely skip attempts to verify correct refcount balancing if the code does not use .DangerousGetAddress(), which will make them easier to read.

{
[StructLayout(LayoutKind.Sequential)]
struct PyObjectStruct
{
#if PYTHON_WITH_PYDEBUG
public IntPtr _ob_next;
public IntPtr _ob_prev;
#endif
public IntPtr ob_refcnt;
public IntPtr ob_type;
public IntPtr ob_dict;
public IntPtr ob_data;
}

public static readonly PyHandle Null = new PyHandle(IntPtr.Zero);

private IntPtr _handle;

public unsafe long RefCount
{
get
{
return (long)((PyObjectStruct*)_handle)->ob_refcnt;
}
set
{
((PyObjectStruct*)_handle)->ob_refcnt = new IntPtr(value);
}
}

public PyHandle(IntPtr op)
{
_handle = op;
}

public PyHandle(long op)
{
_handle = new IntPtr(op);
}

public unsafe PyHandle(void* op)
{
_handle = (IntPtr)op;
}

public override string ToString()
{
// Make the PyHandle be more readable for printing or debugging.
#if !PYTHON2
// Check GIL directly make sure PyHandle can get a string description
// when it didn't hold the GIL.
if (!Runtime.PyGILState_Check())
{
return $"<object at {_handle}>";
}
var s = Runtime.PyObject_Str(_handle);
PythonException.ThrowIfIsNull(s);
try
{
return Runtime.GetManagedString(s);
}
finally
{
Runtime.XDecref(s);
}
#else
// Pytyhon2 didn't has PyGILState_Check, always print its pointer only.
return $"<object at {_handle}>";
#endif
}

public void XIncref()
{
if (_handle == IntPtr.Zero)
{
return;
}
IncrefInternal();
}

public void Incref()
{
if (_handle == IntPtr.Zero)
{
throw new NullReferenceException();
}
IncrefInternal();
}

public void XDecref()
{
if (_handle == IntPtr.Zero)
{
return;
}
DecrefInternal();
}

public void Decref()
{
if (_handle == IntPtr.Zero)
{
throw new NullReferenceException();
}
DecrefInternal();
}

public void Dispose()
{
if (_handle == IntPtr.Zero)
{
return;
}
DecrefInternal();
_handle = IntPtr.Zero;
}

public override bool Equals(object obj)
{
if (!(obj is PyHandle))
{
return false;
}
return _handle == ((PyHandle)obj)._handle;
}

public override int GetHashCode()
{
return _handle.GetHashCode();
}

private unsafe void IncrefInternal()
{
((PyObjectStruct*)_handle)->ob_refcnt = ((PyObjectStruct*)_handle)->ob_refcnt + 1;
}

private unsafe void DecrefInternal()
{
var p = (PyObjectStruct*)_handle;
p->ob_refcnt = p->ob_refcnt - 1;
if (p->ob_refcnt == IntPtr.Zero)
{
IntPtr tp_dealloc = Marshal.ReadIntPtr(p->ob_type, TypeOffset.tp_dealloc);
if (tp_dealloc == IntPtr.Zero)
{
return;
}
NativeCall.Void_Call_1(tp_dealloc, _handle);
}
}

public static bool operator ==(PyHandle a, PyHandle b) => a._handle == b._handle;
public static bool operator !=(PyHandle a, PyHandle b) => a._handle != b._handle;
public static bool operator ==(PyHandle a, IntPtr ptr) => a._handle == ptr;
public static bool operator !=(PyHandle a, IntPtr ptr) => a._handle != ptr;

public static unsafe explicit operator void*(PyHandle handle) => (void*)handle._handle;
public static implicit operator IntPtr(PyHandle handle) => handle._handle;
public static implicit operator PyHandle(IntPtr op) => new PyHandle(op);

public static implicit operator PyHandle(BorrowedReference reference)
=> new PyHandle(reference.DangerousGetAddress());
public static implicit operator PyHandle(NewReference reference)
=> new PyHandle(reference.DangerousGetAddress());
}
}
6 changes: 3 additions & 3 deletions src/runtime/pythonexception.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ namespace Python.Runtime
/// </summary>
public class PythonException : System.Exception, IPyDisposable
{
private IntPtr _pyType = IntPtr.Zero;
private IntPtr _pyValue = IntPtr.Zero;
private IntPtr _pyTB = IntPtr.Zero;
private PyHandle _pyType;
private PyHandle _pyValue;
private PyHandle _pyTB;
private string _tb = "";
private string _message = "";
private string _pythonTypeName = "";
Expand Down
Loading