Skip to content
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

Implementation of from_bytes_multiple() #39

Closed
thomaspenteker opened this issue Jan 28, 2015 · 10 comments
Closed

Implementation of from_bytes_multiple() #39

thomaspenteker opened this issue Jan 28, 2015 · 10 comments

Comments

@thomaspenteker
Copy link

Hello,

would it be possible to provide a function from_bytes_multiple that mimics it read_multiple() counter-part? There are several situations where data may be received via network. Currently there's the option to use os.pipe() to provide file-object based access to the data. This is suboptimal due to constraints on the pipe size. The other to make this work is by writing the data to temporary files and reading it again with read_multiple() which is bad for obvious reasons.

Thanks in advance.

@jparyani
Copy link
Contributor

Added in 88db4ab

You can install with pip install https://github.com/jparyani/pycapnp/archive/develop.zip. Let me know if you'd rather I push a real release/version bump to pypi.

@thomaspenteker
Copy link
Author

Wow, that was a really quick reply, thank you!

Using the new function makes python segfault after some time, though.

This is the full backtrace from the core dump:
https://gist.github.com/thomaspenteker/c88978ae4266a49b4910
The core dump can be found at http://serverop.de/~tek/core.gz

I pulled your changes and ran

setup.py build --debug && pip install .

capnproto is from Jan 4th 2015.

Several files were created before/during the compile step:
schema: https://gist.github.com/thomaspenteker/2e1d1a28dca0b3ec40cf
pycapnp: https://gist.github.com/thomaspenteker/9352f26d76f15a86d70d

The serialized data crashing python can be found at
http://serverop.de/~tek/data.bin.gz

I am not quite sure if this is an issue with pycapn or capnproto itself. Let me know
if you need any further information. capnp can handle the file,

cat data.bin | capnp decode schema Events

works just fine.

@jparyani
Copy link
Contributor

I'm having troubles reproducing on my end. I downloaded schema.capnp and test.py from your first gist as well as data.bin.gz, and ran python test.py. It spit out ran 2 times. Did you do anything differently?

Also, I'm a little surprised you've been using capnp compile -ocython -oc++ schema.capnp. You don't seem to be actually building/using it? The cython code generator is an experimental optimization that I've intentionally left undocumented until I'm sure it's stable enough for wider use :)

@thomaspenteker
Copy link
Author

The compile step was done from the habit of compiling things for the c++ part of capnproto. I did not think too much about it. So I am not using this atm.

I copied the schema and test.py to another machine (different Distro with Python 2.7.9 (default, Dec 12 2014, 18:48:30) [GCC 4.8.3] on linux2) with the same results. Here is what I did:

https://gist.github.com/thomaspenteker/2a8b4b0eb55152e3496e
As an alternative, I used the capnproto 0.5.1 release yielding the exact same results.
The only error I saw was during pip install .:
/root/tmp/tmpcgenzY/root/tmp/timer_createMoxgJn.o: In function main':
timer_createMoxgJn.c:(.text+0x15): undefined reference to timer_create'

it kept going, though.

I also noticed that running %load test.py in ipython and exiting succeeds. BUT
pasting test.py, running it and afterwards deleting x makes ipython core dump, too:

ran 2 times

In [2]: del x

In [3]: [EOF]
zsh: segmentation fault  ipython --no-banner --no-confirm-exit

So calling the destructor capnp::InputStreamMessageReader::~InputStreamMessageReader() seems to be the culprit here.

Can this be an issue in capnproto itself? How could I find the difference between your invocaton and mine? I'm at loss for ideas why this works for you and fails for me.

@jparyani
Copy link
Contributor

Gah so this was due to a subtle issue in deconstructor ordering with Cython/C++. It should be fixed now in develop. Checkout a62e392 to see the changes.

If you're interested in the nitty-gritty details, it arose because InputMessageReader inherits from MessageReader in both the original C++ library and in my Cython wrapping of those classes, and in Cython I keep a reference to an input buffer (usually a python string) in the CythonInputMessageReader subclass. This reference must survive for the lifetime of the underlying C++ InputMessageReader. Because of deconstructor ordering, CythonInputMessageReader is deconstructed before CythonMessageReader, and the buffer is deallocated immediately because CPython garbage collects immediately in cases like this. Then in CythonMessageReader the deconstructor of the C++ MessageReader class is called, but it's virtual and calls InputMessageReader's destructor. This then segfaults, since the buffer it expects to exist has already been deallocated.

It's a subtle issue, and I'm going to have to audit my code to make sure I didn't make this mistake anywhere else. Thankfully, I don't think I used inheritance in this manner very much...

@thomaspenteker
Copy link
Author

This fixed the issue, thank you! + interesting catch wrt the order destructors :)

@JohnEmhoff
Copy link

I'm fairly certain I'm hitting this deallocation issue from an exception being thrown at just the right time -- is master in a good state to cut a new release to pypi and/or can I bribe you to do so? :)

@jparyani
Copy link
Contributor

Alright, I've uploaded a new version (v0.5.2) up to pypi. Please let me know if it solves the issue for you.

@JohnEmhoff
Copy link

Thanks! Unfortunately it still seems to segfault in the same spot. I discovered I can recreate the crash very easily with something like this in my app:

obj = item.from_packed_bytes()
raise ValueError()

...although I can't seem to recreate it in a stripped down context. I managed to run it against Valgrind and it looks like ~PackedMessageReader() is in the call stack so I still think it's related to this issue. I've pasted the Valgrind error log here:

https://gist.github.com/JohnEmhoff/db3b44e0ecf43c3c2d22

In the meantime I'll see if I can reproduce the crash in a smaller program.

@JohnEmhoff
Copy link

Okay, I've got it reproduced in a pretty small program that I've linked to below. Loading up certain messages with from_bytes_packed seems to cause a segfault when the item is GC'd.

To reproduce:
$ wget https://www.dropbox.com/s/x2bjadzzjfll78y/repro.tar.gz
$ tar xvzf repro.tar.gz
$ cd repro
$ python repro.py
segmentation fault

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants