gracinet added a comment.

  @yuja yes sorry, forgot to adapt to the new signature. That'll be fixed in 
next version, and in `revlog.c`, the capsule pointer declaration is now at the 
top.
  
  About protecting parents() with the GIL `Python<'p>`, I'll have to think more 
about it, but the solution that comes to mind is as you suggest a two-step 
wrapping: the long lived `Index` would wrap the actual index object and the 
function pointer retrieved from the capsule, but would not implement the 
`Graph` trait directly. A shorter term `GILProtectedIndex` would then implement 
the `Graph` trait by wrapping the `Index` together with the `Python<'p>` marker 
coming from a call to the Python interface (methods of the `py_class!` of this 
series), hence forbidding to release the GIL from any code that calls 
`Graph::parents()`. I think the current usage is safe, though, because our 
Python interface methods don't release the GIL, but it's true the `Index` isn't 
inherentrly safe.
  
  A simpler possibility would be to go the other way: release the GIL from the 
Python interface methods, and reacquire it at each call of `Graph::parents`. I 
don't know what the overhead would be, especially with Mercurial being 
currently monothreaded (unless I'm mistaken), meaning that we don't have 
benefits to reap. Maybe I'll try that one, since it's so simple, and if I 
measure negligible overhead, I'll go for it.
  
  As a side note, the same problem could arise with the direct-ffi bindings: 
the safety promise is held by the caller, this time from C code.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5438

To: gracinet, #hg-reviewers
Cc: yuja, durin42, kevincox, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to