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

[#8969] Fix for twisted.internet.sendmsg segfault on Linux Python 2 #650

Closed
wants to merge 6 commits into from

Conversation

exvito
Copy link
Contributor

@exvito exvito commented Dec 29, 2016

@codecov-io
Copy link

codecov-io commented Dec 29, 2016

Current coverage is 90.66% (diff: 66.66%)

No coverage report found for trunk at 9ccee5c.

Powered by Codecov. Last update 9ccee5c...08b363e

@rodrigc
Copy link
Contributor

rodrigc commented Dec 29, 2016

Does this fix:
https://twistedmatrix.com/trac/ticket/5649
?

@exvito
Copy link
Contributor Author

exvito commented Dec 29, 2016

@rodrigc,

I can't reproduce the behaviour in https://twistedmatrix.com/trac/ticket/5649 here. With that code, from trunk, I get a segfault and not a SIGABRT. With this PR I get an exception with errno=ENOTSOCK, which seems to make sense, given STDIN is being used.

PS: Quickly tested on an Ubuntu 16.04 virtual machine.

@exvito
Copy link
Contributor Author

exvito commented Dec 29, 2016

Dear reviewers,

I need guidance to find a way of ensuring 100% diff coverage.

Given that the test for "does not segfault" is calling and disregarding the result or acceptable exceptions like socket.error, in this case I wrapped the call in a try/except block that travis is not fully covering. The except block is only hit on Mac OS, confirmed at least 10.9 and 10.10.

Any pointers? Maybe some other strategy is needed?...
Thanks in advance.

@rodrigc rodrigc closed this Dec 29, 2016
@rodrigc
Copy link
Contributor

rodrigc commented Dec 29, 2016

Moved to #651

member to see if the "message" fits in the buffer, and
returns NULL if it doesn't. Zero-filling the buffer
ensures that this doesn't happen. */
memset(message_header.msg_control, 0, all_data_len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyMem_Calloc? Or even PyMem_RawCalloc.

Copy link
Contributor Author

@exvito exvito Dec 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can tell from the Python docs, those are not available on Python 2, for which _sendmsg.c is needed. Am I mistaken?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyMem_Calloc() is new in Python 3.5, and apparently not in Python 2.

@exvito exvito deleted the 8969-exvito-sendmsg-segfault branch December 30, 2016 16:03
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

Successfully merging this pull request may close these issues.

None yet

4 participants