DecRef (in Goroutine) leads to crash #16
Replies: 2 comments 5 replies
-
|
@jshiwam one of the first things I notice is there's no proper error handling, nil checks. This is how I tend do it: detectdataPy := detect.CallObject(args)
if !(detectdataPy != nil && python3.PyErr_Occurred() == nil) { //retval: New reference
python3.PyErr_Print()
return nil, fmt.Errorf("error calling function detect")
}
defer detectdataPy.DecRef()(note that this might not necessarily be the best way to do it, just what I tend to use) |
Beta Was this translation helpful? Give feedback.
-
|
Hi @christian-korneck , thanks for the error handling part, the main issue is something else here. The reason behind the above code crashing while I try to DecRef the PyObject(pyTuple argument in the code that I posted) is as follows: state := python3.PyEval_SaveThread() // release the GIL, the GIL is unlocked for using by goroutines
// Restore the state and lock the GIL
defer python3.PyEval_RestoreThread(state) // acquire the GIL, the GIL is locked by the main thread
args := python3.PyTuple_New(1) // new reference, a call DecRef() is needed
if args == nil && python3.PyErr_Occurred() != nil {
err = errors.New("error creating python tuple object")
return
}
defer args.DecRef()
defer fmt.Println("DecReffing Args", args)
ret := python3.PyTuple_SetItem(args, 0, limit) // steals reference to limit
// Cleans the Go variable, because now a new owner is caring about related PyObject
// no action, such as a call DecRef(), is needed here
limit = nil
if ret != 0 {
err = errors.New("error setting a tuple item")
limit.DecRef()
limit = nil
return
}
// for true {
wg.Add(20)
for i := 0; i < 10; i++ {
go func() {
runtime.LockOSThread()
_gstate := python3.PyGILState_Ensure() // acquire the GIL, the GIL is locked by the 1st goroutine
odds.Call(args, python3.PyDict_New())
python3.PyGILState_Release(_gstate) // release the GIL, the GIL is unlocked for using by others
wg.Done()
}()
go func() {
runtime.LockOSThread()
_gstate := python3.PyGILState_Ensure() // acquire the GIL, the GIL is locked by the 2nd goroutine
even.Call(args, python3.PyDict_New())
python3.PyGILState_Release(_gstate) // release the GIL, the GIL is unlocked for using by others
wg.Done()
}()
}
wg.Wait()
// }Here I have used the code in @esendjer's PR, he probably needs to add As you can see the PyObject Ref Count here is 1. I don't understand why I can't decref a PyObject when the GIL is in unlocked state. Are there any refs that I can go through to understand it? Thanks! |
Beta Was this translation helpful? Give feedback.
-
I have gone through this btw, I am confused on whether this needs to be done in the main thread as well? Because there is no concurrency at the level of main there, there is only one main thread here. |
Beta Was this translation helpful? Give feedback.
-
|
@jshiwam I don't really get your point. Can you post a complete, runnable example? Side note: I noticed that you've copied at least one "mistake" from the (unreviewed) PR code: should probably not check for |
Beta Was this translation helpful? Give feedback.
-
|
Here the above code will fail because there is a DecRef on line number 69 which is DecReffing a PyObject when the GIL is unlocked. My misunderstanding in this code was that, we only need to lock the gil and do python inside go routines, which is not true. Even if it is the main thread we still need to lock the gil before performing python operations. |
Beta Was this translation helpful? Give feedback.
-
|
thanks, I’ll have a look. |
Beta Was this translation helpful? Give feedback.
-
|
I think part of the problem is that in this example you're releasing the GIL in the However, directly after these lines Python stuff is happening in the main function. If you move the above block to the line before you start the Goroutines it doesn't seem to crash: https://go.dev/play/p/ogdAmVyl5F0 Maybe as a tiny bit of advice (maybe it helps you, maybe not). I highly recommend to look up every Python C-API function that you call in the Python C-API docs. Imho it's the only way to avoid deadly surprises. |
Beta Was this translation helpful? Give feedback.
-
originally posted by @jshiwam as comment to #5 .
moved from the PR to github discussions by the mod.
author: @jshiwam
date: 14-Jun-22
Beta Was this translation helpful? Give feedback.
All reactions