-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
Segfault in __pyx_f_5adbus_5sdbus_call_callback #42
Comments
We haven't seen that one before. Do you know what version of libsystemd you have? It could be line 9 of adbus/sdbus/call.pyx, it doesn't look like we are saving the slot reference that returns from an unref which could maybe cause this, you could maybe try changing it to: You'll have to run setup.py with --cythonize to recreate the c file. If you aren't able to get this working / tested we will probably have some time to run some of our own testing in a few days (maybe on the weekend). Thanks! |
239-7ubuntu10.10
This doesn't seem to work, there's no Error compiling Cython file:
------------------------------------------------------------
...
sdbus_h.sd_bus_error *err):
cdef PyObject *call_ptr = <PyObject*>userdata
cdef Call call = <Call>call_ptr
cdef Message message = Message()
self._slot = sdbus_h.sd_bus_slot_unref(self._slot)
^
------------------------------------------------------------
adbus/sdbus/call.pyx:9:43: undeclared name not builtin: self |
I'm sorry, I meant change it to: call._slot = sdbus_h.sd_bus_slot_unref(call._slot) That really is a bug (it may not be causing your issue, but it's a bug non-the-less). I'm going to make that change now and check it it. Let me know if helps with your issue. |
Still seems to segfault with that. The crash seems to be limited to one specific function call in my app: nm_service = adbus.Service(bus='system')
@classmethod
async def get_ipv6_config(cls, path):
config = await adbus.client.get_all(
cls.nm_service,
'org.freedesktop.NetworkManager',
path,
'org.freedesktop.NetworkManager.IP6Config'
)
return config Where When it doesn't crash I get this for {'AddressData': [{'address': 'fe80::7962:5739:c34f:deef', 'prefix': 64}],
'Addresses': [([254,
128,
0,
0,
0,
0,
0,
0,
121,
98,
87,
57,
195,
79,
222,
239],
64,
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0])],
'DnsOptions': [],
'DnsPriority': 100,
'Domains': [],
'Gateway': '',
'Nameservers': [],
'RouteData': [{'dest': 'fe80::', 'metric': 100, 'prefix': 64},
{'dest': 'ff00::', 'metric': 256, 'prefix': 8, 'table': 255}],
'Routes': [([254, 128, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
64,
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
100)],
'Searches': []} This is what I get when using
|
It's possible that we don't need line 9 of sdbus/call.pyx anymore. Some of the systemd reference code has it, and some doesn't, and it looks like there may be an additional unref after the callback is called. You could try removing it, though, I suspect you will just push the crash a few lines further up the stack, or after the return from the callback. |
Or, it's possible python is freeing the Call instance before the callback is called, to be safe we should probably increment before the callback, and then decrement after the callback, the Python reference counter for the Call object. I'll look into that. |
Yeah, I'm thinking it's likely something like that, I'm not super familiar with cython myself though. Would adding a check on if call._slot:
sdbus_h.sd_bus_slot_unref(call._slot) |
That probably isn't going to help, if call is removed by the Python Garbage Collector that memory may not be zeroed, and the library call runs the same check anyway. The only time that it's a possibility that the Python GC has removed it is if a timeout has occurred, that's definitely a bug though, but it may not be causing your issue. I'll add reference count increments / decrement and we can see if it fixes it. |
I just made this update, maybe give it a try again. |
I'm going to close this for now, my developer who was seeing this segfault hasn't seen it happen on the current master branch, although due to the intermittent nature it's hard to say for sure it's fixed. Seems either your fix or my fix for the Invalid read bug fixed it. |
Thanks, hopefully we got it. If it comes back feel free to re-open. |
Under some environments(such as ubuntu) I seem to get this segfault. Any idea why that would be happening?
backtrace:
The text was updated successfully, but these errors were encountered: