Feature request: share and/or cache message connections to remote clients

[jpdillingham]

Describe the solution you'd like

Presently, Nicotine+ attempts to negotiate a message connection for each message (at least, each transfer request) it tries to send. Negotiating a single connection and then reusing it to send subsequent messages until it is disconnected would improve performance and help prevent socket exhaustion.

In addition to potential performance gains, I'm also selfishly requesting this change to improve interoperability between Nicotine+ and my Soulseek.NET library.

Additional context

The official Soulseek clients happily supercede existing connections when a new one is established, but when connections are superseded you risk losing messages that were in flight (depending on how concurrent this operation is in your application, this may not be a big deal for Nicotine+).

With Soulseek.NET, I maintain a connection cache which only supersedes connections when a new one is established directly by way of a connection on the listening port (official clients consider this superceded on their end so this isn't my choice to make). For 'connect to peer' requests, if a connection is cached then the connect to peer request is ignored. This works well with Soulseek NS and QT.

Presently, when requesting the download of multiple files from Soulseek.NET from Nicotine+, I receive a 'connect to peer' request for each file requested, but because I only allow one connection at a time to connect, I only get one transfer request. The subsequent requests fail with a connection error, after waiting in "Getting address" for some time. There's a compounding issue regarding direct/indirect connections where only an indirect connection is attempted if the "I can accept direct connections" is ticked in settings. Attempting both types of connections by default would solve this issue when the remote client can accept direct connections.

I could solve this problem by allowing 'connect to peer' requests to supercede established connections, but in some limited testing this morning I experienced some dropped messages (due to concurrency issues between message handling and connection hand-off), and I'm concerned about issues that would arise when receiving search results with this enabled with regard to socket exhaustion.

[kiplingw]

Thanks @jpdillingham. I think there a number of reasons to recycle existing connections. You've raised some good ones. If that was fixed, I have a sneaking suspicion that a lot of connection and transfer issues people have been having over the years may disappear.

I'm not sure if we are re-allocating sockets too, but if that's the case, the first thing that came to my mind was that they can be an expensive resource to allocate on some platforms.

[mathiascode]

Peer connectivity is definitely an area that should be improved, the code related to direct/indirect connections in particular should be carefully reviewed. I've never liked the fact that Nicotine+ contains the "I can receive direct connections" checkbox, as it has already resulted in bug reports because it was set to the wrong mode.

[mathiascode]

@jpdillingham I believe I've fixed this issue in the following branch:
git clone -b peerwork https://github.com/nicotine-plus/nicotine-plus

Testing would be appreciated, to ensure I haven't screwed something up.

[hboetes]

We've just finished testing this problem. Seems to work. 👍

[mathiascode]

Switched to a new branch:
git clone -b peerconnectionimprovements https://github.com/nicotine-plus/nicotine-plus

Windows builds if necessary (use 32-bit for now, 64-bit is broken):
https://github.com/mathiascode/nicotine-plus/actions/runs/342552205

[jpdillingham]

@mathiascode great! I'll take a look in a bit.

[jpdillingham]

@mathiascode getting (looks truncated?):

Failed to execute script pyi_rth_certifi

[mathiascode]

64-bit Windows builds broke recently due to some package updates, I will fix them before the next release. Until then, use the 32-bit build.

[jpdillingham]

@mathiascode Works!

[mathiascode]

Great, thanks for testing!

[mathiascode]

If anyone else reads this issue in the future, here's some backstory I found for the previous "I can receive direct connections" option: http://web.archive.org/web/20100802111948/http://nicotine.thegraveyard.org/cgi-bin/faqw.py?req=show&file=faq04.002.htp

[hboetes]

Perhaps add a FAQ document in the wiki?

[mathiascode]

Since we don't have that option anymore, I don't think that's necessary. I'd rather avoid FAQs as much as possible, since many users don't seem to read these in my experience. Things should be more obvious in the UI, or have a small explanation next to them in the UI, if necessary.