-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Fix string type handling in attach_kprobe() #623
Comments
chantra
added a commit
to chantra/bcc
that referenced
this issue
Jul 23, 2016
* pass -fno-color-diagnostics to clang * remove unicode import (iovisor#623) * add time to cachetop output * add keybindings to cachetop.8 * add cachetop links to README.md
chantra
added a commit
to chantra/bcc
that referenced
this issue
Jul 23, 2016
* pass -fno-color-diagnostics to clang * remove unicode import (iovisor#623) * add time to cachetop output * add keybindings to cachetop.8 * add cachetop links to README.md
markdrayton
referenced
this issue
Jul 27, 2016
Prior to this diff we used inconsistent types for keys in `open_kprobes`. The results from the regex match (`attach_kprobe(event_re=..)`) and the automatic `kprobe__` features were passed through `str.decode()`, yielding unicode keys, but specific matches (i.e. from `attach_kprobe(event=..)`) were stored with string keys passed down from the caller. Only probes under string keys were released in `cleanup_kprobes`, leaving attached probes on program exit. This diff makes all the keys regular strings. I erred on the side of using regular strings over `str.decode()`ing them because a) this data isn't passed outside of Python, b) it's more Python 3 compatible (there is no `.decode()` on a regular string object in Python 3 so such a change would ultimately need removing again). I also cleaned up a few other things: * removed the call to `awk` for getting probable functions * removed the `isinstance` checks when cleaning uprobes/tracepoints -- we should only have string keys in these dicts * made `num_open_kprobes` skip the perf_events buffers. People likely use this to check that the right number of probes have been placed so counting perf_events buffers doesn't make sense here
closed via #632. |
palmtenor
pushed a commit
to palmtenor/bcc
that referenced
this issue
Jul 31, 2016
* pass -fno-color-diagnostics to clang * remove unicode import (iovisor#623) * add time to cachetop output * add keybindings to cachetop.8 * add cachetop links to README.md
palmtenor
pushed a commit
to palmtenor/bcc
that referenced
this issue
Jul 31, 2016
`open_kprobes` is a dict of open kprobes. Its keys are strings for normal probes and a tuple for perf buffers. Normal probes need unregistering on script exit; perf buffers do not. `cleanup` currently looks for string keys (specifically type `str`) when working out what to unregister, which is a bit brittle -- in Python2 strings can be both native `str` and `unicode`, depending what exactly was passed to `attach-*/detach_*` and whether `from __future__ import unicode_literals` is used (e.g. iovisor#623). This diff makes the API more relaxed by casting the probe name to `str` to match the expectations of `cleanup`. This works in py2 (with and without unicode_literals) and py3.
chantra
pushed a commit
to chantra/bcc
that referenced
this issue
Sep 16, 2016
`open_kprobes` is a dict of open kprobes. Its keys are strings for normal probes and a tuple for perf buffers. Normal probes need unregistering on script exit; perf buffers do not. `cleanup` currently looks for string keys (specifically type `str`) when working out what to unregister, which is a bit brittle -- in Python2 strings can be both native `str` and `unicode`, depending what exactly was passed to `attach-*/detach_*` and whether `from __future__ import unicode_literals` is used (e.g. iovisor#623). This diff makes the API more relaxed by casting the probe name to `str` to match the expectations of `cleanup`. This works in py2 (with and without unicode_literals) and py3.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
As discovered in #615,
attach_kprobe()
withfrom __future__ import unicode_literals
makes an assertion failure:The assert exists because we later use the key type to determine how to unregister a kprobe:
bcc/src/python/bcc/__init__.py
Line 770 in f4bf275
We can probably make this more robust by splitting the probe types or filtering in a way that matches all string types.
The text was updated successfully, but these errors were encountered: