Bug 1379863

+++ This bug was initially created as a clone of Bug #1367216 +++

Description of problem:
After using nss.io.Socket.import_tcp_socket, the moment I try to read from the imported socket, I get a ValueError: I/O operation on closed socket.

Version-Release number of selected component (if applicable):
python-nss-1.0.0-beta1.2.fc24.1

How reproducible:
Consistent

Steps to Reproduce:
1. Run the below code

Actual results:
Traceback (most recent call last):
  File "test.py", line 31, in <module>
    test()
  File "test.py", line 29, in test
    print c1_nss.recv(1024)
ValueError: I/O operation on closed socket


Expected results:
Hello world

Additional info:
This works on the same machine if I downgrade python-nss to 0.16.0.


Code:
import socket
import nss
import nss.ssl
import nss.io

def _tcp_socketpair():
    '''Like socket.socketpair(), but using AF_INET sockets.

    This is necessary because NSS uses getsockname() to create session
    identifiers and it does not support AF_UNIX sockets created by
    socket.socketpair().'''

    server = socket.socket(socket.AF_INET, socket.SOCK_STREAM,
                           socket.IPPROTO_TCP)
    server.bind(('', 0))
    server.listen(1)
    client = socket.socket(socket.AF_INET, socket.SOCK_STREAM,
                           socket.IPPROTO_TCP)
    client.connect(server.getsockname())
    (server2, _) = server.accept()
    server.close()
    return (client, server2)

def test():
    (c1, s1) = _tcp_socketpair()
    c1_nss = nss.io.Socket.import_tcp_socket(c1.fileno())
    s1.sendall('hello world')
    print c1_nss.recv(1024)

test()

--- Additional comment from Patrick Uiterwijk on 2016-08-15 17:48:42 EDT ---



--- Additional comment from Patrick Uiterwijk on 2016-08-15 18:14:02 EDT ---

It's not just import_tcp_socket that's broken. The nss.io.Socket.new_tcp_pair is also broken.
This reproducer has the same effect:


import nss.io

(c1, s1) = nss.io.Socket.new_tcp_pair()
s1.sendall('hello world')
print c1.recv(1024)

--- Additional comment from John Dennis on 2016-08-16 12:25:19 EDT ---

Mostly making notes for myself in this comment.

I built a scratch version for Patrick which removed all the socket state tracking code and checks for the socket being open (patch attached: remove_open_for_read.patch) and that solved his problem, but it's probably not the correct solution.

The socket state code was added to address a segfault identified in bug #1229748

It's obvious to me now that trying to track the state of a socket is futile, especially since external fd's can be passed in and manipulated outside of the python binding. The central issue seems to be whether pr_socket should be set to NULL on close and where and when PR Socket resources are cleaned up.

We still need to protect against a segfault so we either need to not set pr_socket to null in the python Socket object when close is called, or we need similar code to SOCKET_CHECK_OPEN that only tests if pr_socket is NULL.

I need to check what happens when PR_Close() is called, is the handle still valid? Who cleans up the resources for the PR Socket? Is there a separate call for that or does that happen when PR_Close() is called? I probably knew the answers at one point but the code in question was written so many years ago I now forget. The answer dictates whether Socket_close() should set pr_socket to NULL or not. There does not seem to be any code in Socket_delloc() that closes the socket and frees PR resources. That looks like an oversight and potential leak.

--- Additional comment from John Dennis on 2016-08-16 12:28 EDT ---

trial patch to see if it solves Patrick's problem, it's not a correct solution but has elements of what needs to be adjusted. Mostly here to identify all the code locations that need to be looked at.

--- Additional comment from Patrick Uiterwijk on 2016-09-19 12:11:15 EDT ---



--- Additional comment from Patrick Uiterwijk on 2016-09-19 12:13:35 EDT ---

Could this please be backported to supported Fedora releases?

--- Additional comment from Fedora Update System on 2016-09-22 05:55:17 EDT ---

python-nss-1.0.0-1.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-189a2448ae

--- Additional comment from Fedora Update System on 2016-09-23 04:01:08 EDT ---

python-nss-1.0.0-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-1801ff25bd

--- Additional comment from Fedora Update System on 2016-09-25 16:56:10 EDT ---

python-nss-1.0.0-1.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.

--- Additional comment from John Dennis on 2016-09-27 18:47:05 EDT ---

re comment #6, this bz was filed against rawhide, I'm cloning it into f24 and will update f24. f23 cannot be updated because f23 is still at the 0 major release version and we're not supposed to bump major versions.