allow execution of on_unknown_torrent method in the absence of active torrents

[joriscarrier]

No description provided.

[arvidn]

I would prefer to keep optimized cases (early exits) in the common case, where there's no plugin. What do you think of adding another feature flag letting the plugin request having on_unknown_torrent be called on it instead?

i.e. add another flag here: https://github.com/arvidn/libtorrent/blob/RC_2_0/include/libtorrent/extensions.hpp#L198-L212

And instead of removing those checks, only disregard them if at least one plugin has requested this feature.

[glassez]

@arvidn
Could you explain what is the purpose of on_unknown_torrent? When is it called?

[joriscarrier]

@arvidn Could you explain what is the purpose of on_unknown_torrent? When is it called?

on_unknown_torrent is called when a connection is made to a torrent that is not found in the session, allowing extensions to determine whether or not a torrent should be added to the session.

[glassez]

a connection is made to a torrent that is not found in the session

How can this be possible? Could you describe the appropriate scenario?

[joriscarrier]

a connection is made to a torrent that is not found in the session

How can this be possible? Could you describe the appropriate scenario?

if you want to simulate this you need 2 sessions you add a torrent to one of them and from the torrent_handle you connect to the other session (via the listening port).

th = session.add_torrent(atp);
th.connect_peer(lt::tcp::endpoint(lt::make_address(ip), port));

the other session will receive a new connection and attempt to attach it to a torrent:
session_impl::incoming_connection --> bt_peer_connection::on_receive_impl --> peer_connection::attach_to_torrent

[glassez]

if you want to simulate this you need 2 sessions you add a torrent to one of them and from the torrent_handle you connect to the other session (via the listening port)

I asked about supposed real scenario. Shouldn't peer query for torrent only those peers that announced that torrent in any way?

[joriscarrier]

I asked about supposed real scenario. Shouldn't peer query for torrent only those peers that announced that torrent in any way?

this is useful if you don't want to keep the torrents in the session but put them back if a peer tries to connect to you for a torrent you previously announced.

[glassez]

I asked about supposed real scenario. Shouldn't peer query for torrent only those peers that announced that torrent in any way?

this is useful if you don't want to keep the torrents in the session but put them back if a peer tries to connect to you for a torrent you previously announced.

What's the point?
And doesn't the previous announce stop when you delete the torrent from the session?

(I'm just trying to figure out if there is any tangible benefit from this approach. Or maybe it's just related to some very specific use case.)

[joriscarrier]

What's the point?

And doesn't the previous announce stop when you delete the torrent from the session?

(I'm just trying to figure out if there is any tangible benefit from this approach. Or maybe it's just related to some very specific use case.)

You can always continue to manually announce a torrent in the dht (session_handle::dht_announce) (I don't know if there is a lifetime for other announcements like lsd, tracker).

This may make it possible not to use resources unnecessarily (keep torrents inactive in the session).

This pull request is only intended to make this function already existing in the case where there is no torrent in the session (before this pull request if you already had an active torrent in your session if a peer tried to connect to a torrent that you did not have in the session the call to on_unknown_torrent would have been made).

[glassez]

This pull request is only intended to make this function already existing in the case where there is no torrent in the session

👍
Given the meaning of this function, I don't understand why it initially depended on the presence of torrents in the session. So this patch definitely makes sense.

[arvidn]

@joriscarrier does this compile for you?
CI is failing with:

rc/session_impl.cpp:3092:43: error: ‘m_ses_extensions’ was not declared in this scope; did you mean ‘SCT_set1_extensions’?
 3092 |                 if (m_torrents.empty() && m_ses_extensions[plugins_unknown_torrent_idx].empty())
      |                                           ^~~~~~~~~~~~~~~~
      |                                           SCT_set1_extensions
src/session_impl.cpp:3147:93: error: ‘m_ses_extensions’ was not declared in this scope; did you mean ‘SCT_set1_extensions’?
 3147 |                 if (!m_settings.get_bool(settings_pack::incoming_starts_queued_torrents) && m_ses_extensions[plugins_unknown_torrent_idx].empty())
      |                                                                                             ^~~~~~~~~~~~~~~~
      |                                                                                             SCT_set1_extensions

[arvidn]

Could you explain what is the purpose of on_unknown_torrent?

One example is if you seed more torrents than you can have active in the session (say, many millions), you can decide to load and add a torrents to the session on-demand. .torrent files have support for a peers field, where you can hard code some peers that you expect to seed the torrent.

Another example might be a crawler, that just wants to index any torrents it learns about, it might want to add the torrent (as a magnet link).

[arvidn]

@joriscarrier it looks like the extension code isn't wrapped in the appropriate defines: https://github.com/arvidn/libtorrent/actions/runs/6912324402/job/18807956001?pr=7537#step:7:432

[joriscarrier]

@joriscarrier does this compile for you? CI is failing with:

rc/session_impl.cpp:3092:43: error: ‘m_ses_extensions’ was not declared in this scope; did you mean ‘SCT_set1_extensions’?
 3092 |                 if (m_torrents.empty() && m_ses_extensions[plugins_unknown_torrent_idx].empty())
      |                                           ^~~~~~~~~~~~~~~~
      |                                           SCT_set1_extensions
src/session_impl.cpp:3147:93: error: ‘m_ses_extensions’ was not declared in this scope; did you mean ‘SCT_set1_extensions’?
 3147 |                 if (!m_settings.get_bool(settings_pack::incoming_starts_queued_torrents) && m_ses_extensions[plugins_unknown_torrent_idx].empty())
      |                                                                                             ^~~~~~~~~~~~~~~~
      |                                                                                             SCT_set1_extensions

I didn't check if TORRENT_DISABLE_EXTENSIONS was defined or not.

I updated it like this:

bool want_on_unknown_torrent = false;
#ifndef TORRENT_DISABLE_EXTENSIONS
want_on_unknown_torrent = !m_ses_extensions[plugins_unknown_torrent_idx].empty();
#endif

[arvidn]

looks good. there should be an entry in the changelog too. especially since it changes behavior of plugins

[arvidn]

I re-triggered CI on top of the latest version of RC_2_0 with the Mac python CI fixed