test_web_seed_http and test_url_seed

[namtsui]

Please provide the following information

libtorrent version (or branch): 1.2.3

platform/architecture: OpenBSD/amd64

compiler and compiler version: clang 8.0.1

please describe what symptom you see, what you would expect to see instead and
how to reproduce it.

I have patches to get test_web_seed_http* and test_url_seed unit tests to pass. unit test report on OpenBSD

In libtorrent-rasterbar, two types of HTTP seeding are supported. "URL
seed" implements BEP 19 and "HTTP seed" implements BEP 17. HTTP seeding
is when a HTTP server assists in the seeding.

Sources:
https://www.libtorrent.org/manual.html#http-seeding
http://www.getright.com/seedtorrent.html
http://bittorrent.org/beps/bep_0019.html
http://bittorrent.org/beps/bep_0017.html

test_url_seed tries to run:

• url_seed_ssl_keepalive
• url_seed_ssl
• url_seed_keepalive
• url_seed
• url_seed_keepalive_rename

test_url_seed manages to run url_seed_ssl_keepalive correctly and then once it gets to url_seed_ssl, which does not use keepalive, it silently returns. I found that it quits at send_header("Connection", "close").

`patch-test_web_server_py' fixes url seed tests as explained in the
diff. This allows the test_url_seed test to "pass," but I am very unsure of this patch.

• What does keepalive do?
• Does my patch defeat the purpose of the test?

Index: patches/patch-test_web_server_py
===================================================================
RCS file: patches/patch-test_web_server_py
diff -N patches/patch-test_web_server_py
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ patches/patch-test_web_server_py	30 Dec 2019 01:00:45 -0000
@@ -0,0 +1,20 @@
+$OpenBSD$
+
+Needed for test_url_seed because tests without keepalive (url_seed_ssl and
+url_seed) quit at send_header("Connection", "close"), causing the unit test to
+fail.
+
+Index: test/web_server.py
+--- test/web_server.py.orig
++++ test/web_server.py
+@@ -164,10 +164,6 @@ class http_handler(BaseHTTPRequestHandler):
+                 s.send_header('Content-Length', end_range - start_range)
+                 if filename.endswith('.gz'):
+                     s.send_header('Content-Encoding', 'gzip')
+-                if not keepalive:
+-                    s.send_header("Connection", "close")
+-                    if not use_ssl:
+-                        s.request.shutdown(socket.SHUT_RD)
+ 
+                 s.end_headers()
+ 

test_web_seed_http tests fail. `patch-test_http_py' fixes these by making the socket timeout by the default "None." This makes it blocking. I feel more confident about this patch.
https://docs.python.org/3/library/socket.html#socket-timeouts

Index: patches/patch-test_http_py
===================================================================
RCS file: patches/patch-test_http_py
diff -N patches/patch-test_http_py
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ patches/patch-test_http_py	30 Dec 2019 01:00:45 -0000
@@ -0,0 +1,18 @@
+$OpenBSD$
+
+Needed for test_web_seed_http and test_web_seed_http_pw to pass. socket has
+soc.settimeout(None) by default.
+
+https://docs.python.org/3/library/socket.html#socket-timeouts
+
+Index: test/http.py
+--- test/http.py.orig
++++ test/http.py
+@@ -217,7 +217,6 @@ def start_server(host='localhost', port=8080, IPv6=Fal
+     else:
+         soc_type = socket.AF_INET
+     soc = socket.socket(soc_type)
+-    soc.settimeout(120)
+     print("PROXY - Serving on %s:%d." % (host, port))  # debug
+     print('python version: %s' % sys.version_info.__str__())
+     soc.bind((host, port))

[arvidn]

the second patch seems reasonable. I would prefer to set the timeout to much higher value though. I'm worried about tests leaving the python process running indefinitely otherwise.

The first test, as you suspect, disables the test for HTTP/1 servers. i.e. where the server closes the connection after each request. What exactly happens to cause the test to fail? is it the python program that terminates on the line s.send_header("Connection", "close")?

[namtsui]

I resolved test_url_seed.

• url_seed_ssl_keepalive
• url_seed_ssl
• url_seed_keepalive
• url_seed
• url_seed_keepalive_rename

It successfully runs url-seed_ssl_keepalive. Then, at url_seed_ssl it throws a SIGPIPE, so it never gets to run any test after url_seed_ssl.
85 - test_url_seed (SIGPIPE)

I investigated in gdb and here are the relevant parts of the backtrace with "Connection reset by peer."

Thread 2 received signal SIGPIPE, Broken pipe.                                                                                                                 
[Switching to thread 276987]                                                                                                                                   
_thread_sys_sendmsg () at -:3                                                                                                                                  
3       -: No such file or directory.                                                                                                                          
(gdb) bt                                                                                                                                                       
#0  _thread_sys_sendmsg () at -:3                                                                                                                              
#1  0x00000b18235263ae in _libc_sendmsg_cancel (s=15, msg=0xb17e40455b0,                                                                                       
    flags=0) at /usr/src/lib/libc/sys/w_sendmsg.c:27                                                                                                           
#2  0x00000b17e925cede in boost::asio::detail::socket_ops::send (s=15,                                                                                         
    bufs=0xb17e4045710, count=1, flags=0, ec=...)                                                                                                              
    at /usr/local/include/boost/asio/detail/impl/socket_ops.ipp:1184                                                                                           
#3  0x00000b17e925cb85 in boost::asio::detail::socket_ops::non_blocking_send (                                                                                 
    s=15, bufs=0xb17e4045710, count=1, flags=0, ec=...,                                                                                                        
    bytes_transferred=@0xb182ca9ce28: 0)                                                                                                                       
    at /usr/local/include/boost/asio/detail/impl/socket_ops.ipp:1258
...
#17 0x00000b17e963b7c8 in libtorrent::aux::async_shutdown (s=..., holder=...)                                                                                  
    at /usr/ports/pobj/libtorrent-rasterbar-1.2.3/libtorrent-rasterbar-1.2.3/src/socket_type.cpp:184                                                           
#18 0x00000b17e938f308 in libtorrent::peer_connection::disconnect (this=0xb17e8a5b730, ec=..., op=libtorrent::sock_read, error=...)                            
    at /usr/ports/pobj/libtorrent-rasterbar-1.2.3/libtorrent-rasterbar-1.2.3/src/peer_connection.cpp:4418                                                      
#19 0x00000b17e9432037 in libtorrent::web_peer_connection::disconnect (this=0xb17e8a5b730, ec=..., op=libtorrent::sock_read, error=...)                        
    at /usr/ports/pobj/libtorrent-rasterbar-1.2.3/libtorrent-rasterbar-1.2.3/src/web_peer_connection.cpp:293                                                   
#20 0x00000b17e9395d44 in libtorrent::peer_connection::on_receive_data (this=0xb17e8a5b730, error=..., bytes_transferred=0)                                    
    at /usr/ports/pobj/libtorrent-rasterbar-1.2.3/libtorrent-rasterbar-1.2.3/src/peer_connection.cpp:5846
...

(gdb) f 19                                                                                                                                                     
#19 0x00000b17e9432037 in libtorrent::web_peer_connection::disconnect (this=0xb17e8a5b730, ec=..., op=libtorrent::sock_read, error=...)                        
    at /usr/ports/pobj/libtorrent-rasterbar-1.2.3/libtorrent-rasterbar-1.2.3/src/web_peer_connection.cpp:293
293             peer_connection::disconnect(ec, op, error);
(gdb) print ec
$1 = (const libtorrent::error_code &) @0xb17e40472b0: {m_val = 54, m_cat = 0xb17ce40b010 <boost::system::system_category()::system_category_const>}
(gdb) print ec.message()
$4 = {<std::__1::__basic_string_common<true>> = {<No data fields>}, static __short_mask = 1, static __long_mask = 1, 
  __r_ = {<std::__1::__compressed_pair_elem<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::__rep, 0, false>> = {
      __value_ = {{__l = {__cap_ = 49, __size_ = 24, __data_ = 0xb17fa37f980 "Connection reset by peer"}, __s = {{__size_ = 49 '1', __lx = 49 '1'}, 
            __data_ = "\000\000\000\000\000\000\000\030\000\000\000\000\000\000\000\200\371\067\372\027\v\000"}, __r = {__words = {49, 24, 
              12197610125696}}}}}, <std::__1::__compressed_pair_elem<std::__1::allocator<char>, 1, true>> = {<std::__1::allocator<char>> = {<No data fields>}, <No data fields>}, <No data fields>}, static npos = 18446744073709551615}

I patched OpenBSD boost asio's socket_ops.ipp. It uses MSG_NOSIGNAL in the flags to sendmsg. From sendmsg(2):

MSG_NOSIGNAL is used to request not to send the SIGPIPE signal if an attempt to send is made on a socket that is shut down for writing or no longer connected.

This gracefully handles the HTTP/1 server behavior of closing the connection after each request. Thanks for this helpful hint. With this test_url_seed passes.

/usr/local/include/boost/asio/detail/impl/socket_ops.ipp

signed_size_type send(socket_type s, const buf* bufs, size_t count,
    int flags, boost::system::error_code& ec)
{
  clear_last_error();
#if defined(BOOST_ASIO_WINDOWS) || defined(__CYGWIN__)
  // Send the data.
  DWORD send_buf_count = static_cast<DWORD>(count);
  DWORD bytes_transferred = 0;
  DWORD send_flags = flags;
  int result = error_wrapper(::WSASend(s, const_cast<buf*>(bufs),
        send_buf_count, &bytes_transferred, send_flags, 0, 0), ec);
  if (ec.value() == ERROR_NETNAME_DELETED)
    ec = boost::asio::error::connection_reset;
  else if (ec.value() == ERROR_PORT_UNREACHABLE)
    ec = boost::asio::error::connection_refused;
  if (result != 0)
    return socket_error_retval;
  ec = boost::system::error_code();
  return bytes_transferred;
#else // defined(BOOST_ASIO_WINDOWS) || defined(__CYGWIN__)
  msghdr msg = msghdr();
  msg.msg_iov = const_cast<buf*>(bufs);
  msg.msg_iovlen = static_cast<int>(count);
#if defined(__linux__)
  flags |= MSG_NOSIGNAL;
#endif // defined(__linux__)
  signed_size_type result = error_wrapper(::sendmsg(s, &msg, flags), ec);
  if (result >= 0)
    ec = boost::system::error_code();
  return result;
#endif // defined(BOOST_ASIO_WINDOWS) || defined(__CYGWIN__)
}

patched with:

$OpenBSD$

Index: boost/asio/detail/impl/socket_ops.ipp
--- boost/asio/detail/impl/socket_ops.ipp.orig
+++ boost/asio/detail/impl/socket_ops.ipp
@@ -1178,7 +1178,7 @@ signed_size_type send(socket_type s, const buf* bufs,
   msghdr msg = msghdr();
   msg.msg_iov = const_cast<buf*>(bufs);
   msg.msg_iovlen = static_cast<int>(count);
-#if defined(__linux__)
+#if defined(__linux__) || defined(__OpenBSD__)
   flags |= MSG_NOSIGNAL;
 #endif // defined(__linux__)
   signed_size_type result = error_wrapper(::sendmsg(s, &msg, flags), ec);

[/usr/ports/pobj/libtorrent-rasterbar-1.2.3/libtorrent-rasterbar-1.2.3/test/test_url_seed.cpp.url_seed_ssl_keepalive   ] ***PASS***
[/usr/ports/pobj/libtorrent-rasterbar-1.2.3/libtorrent-rasterbar-1.2.3/test/test_url_seed.cpp.url_seed_ssl             ] ***PASS***
[/usr/ports/pobj/libtorrent-rasterbar-1.2.3/libtorrent-rasterbar-1.2.3/test/test_url_seed.cpp.url_seed_keepalive       ] ***PASS***
[/usr/ports/pobj/libtorrent-rasterbar-1.2.3/libtorrent-rasterbar-1.2.3/test/test_url_seed.cpp.url_seed                 ] ***PASS***
[/usr/ports/pobj/libtorrent-rasterbar-1.2.3/libtorrent-rasterbar-1.2.3/test/test_url_seed.cpp.url_seed_keepalive_rename] ***PASS***

Unfortunately, test_web_seed_http still fails when I extend the timeout. I will keep poking at it.

         91 - test_web_seed_http (SIGPIPE)
         92 - test_web_seed_http_pw (SIGPIPE)

[arvidn]

ah, interesting. My impression is that asio is supposed to disable SIGPIPE (and it seems like an oversight to not have done so on BSD). It doesn't seem appropriate for libtorrent to install a (global) signal handler, as a library, but I suppose the tests could be made to ignore SIGPIPE.

@namtsui please upstream this patch to asio. Also, do you know if there are any flavours or older versions of BSD (worth considering) that do not support the MSG_NOSIGNAL flag? It's probably a good idea to look for other calls to ::sendmsg() or other mentions of MSG_NOSIGNAL that also are excluded from __OpenBSD__ that shouldn't.

@chriskohlhoff ping, seems like an asio issue on BSD, I suspect other flavours too.

[arvidn]

I think this issue may be related to the memory leak reported here: qbittorrent/qBittorrent#12326