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

WIP: add callbacks to alerts (should I continue?) #7557

Open
wants to merge 3 commits into
base: RC_2_0
Choose a base branch
from

Conversation

joriscarrier
Copy link
Contributor

this pull request allows you to have a callback (which will be executed on the libtorrent thread when the alert is set in the queue) on asynchronous methods that trigger alerts:

torrent_handle:

  • add_piece
  • read_piece
  • post_peer_info
  • post_status
  • post_download_queue
  • post_trackers
  • set_metadata
  • flush_cache
  • force_recheck
  • save_resume_data
  • post_piece_availability
  • prioritize_files
  • file_priority
  • scrape_tracker
  • move_storage
  • rename_file

session_handle:

  • post_torrent_updates
  • post_session_stats
  • post_dht_stats
  • async_add_torrent
  • dht_live_nodes
  • dht_sample_infohashes
  • dht_direct_request
  • remove_torrent

@joriscarrier
Copy link
Contributor Author

joriscarrier commented Nov 18, 2023

a simple example:

std::mutex m;
std::condition_variable cv;

bool done = false;
th.flush_cache([&cv, &done] (const lt::cache_flushed_alert* alert) {
    // here you are in the libtorrent network thread
    done = true;
    cv.notify_one();
});

{
    std::unique_lock lk(m);
    while (!done) cv.wait(lk);
}

@glassez
Copy link
Contributor

glassez commented Nov 18, 2023

For me, it doesn't look too useful to use callbacks to get some requested data from libtorrent, which will simply be used in another thread (existing alerts will also be quite suitable here).
For me, it looks much more useful to be able to set callbacks for some events that occurred during the torrenting process (in general, not initiated from the user side, for example, when metadata is received, torrent finished etc.), so that you can make some immediate impact on libtorrent itself, which can be too late to do during regular alert processing in the application thread. And (I don't remember if I mentioned this earlier in another discussion), it shouldn't be tied to alerts. It just has to be some key events (on_metadata_received, on_torrent_finished or more generic on_torrent_state_changed, etc.) that can also generate alerts or may not do so. In any case, this should be regardless of the current alert subscription.

@joriscarrier
Copy link
Contributor Author

For me, it looks much more useful to be able to set callbacks for some events that occurred during the torrenting process (in general, not initiated from the user side, for example, when metadata is received, torrent finished etc.), so that you can make some immediate impact on libtorrent itself

looks like you want to use a torrent_plugin ?

my proposal is based on the fact that without a similar modification on the client side (at the level of torrent_handle or session_handle) you cannot identify which alerts correspond to which calls.

@glassez
Copy link
Contributor

glassez commented Nov 18, 2023

looks like you want to use a torrent_plugin ?

I have to use libtorrent extensions feature (torrent_plugin etc.) currently to workaround it.
I just want it to be more powerful and convenient.

@glassez
Copy link
Contributor

glassez commented Nov 18, 2023

my proposal is based on the fact that without a similar modification on the client side (at the level of torrent_handle or session_handle) you cannot identify which alerts correspond to which calls.

Agree. It makes sense in case when some alert is reaction to user requested action.

@joriscarrier
Copy link
Contributor Author

looks like you want to use a torrent_plugin ?

I have to use libtorrent extensions feature (torrent_plugin etc.) currently to workaround it. I just want it to be more powerful and convenient.

like a torrent_handle::wait(std::function<void(lt::torrent_status)>) feature ??

@glassez
Copy link
Contributor

glassez commented Nov 19, 2023

like a torrent_handle::wait(std::function<void(lt::torrent_status)>) feature ??

Something like void session_handle::on_torrent_state_changed(std::function<void (torrent_handle, torrent_status::state_t)>); (where second parameter of callback is previous state)

@joriscarrier joriscarrier force-pushed the callback_alert branch 3 times, most recently from 0505943 to 79858f5 Compare November 21, 2023 07:31
@e1y4s
Copy link

e1y4s commented Jan 5, 2024

That would be a super interesting feature to simplify the coding workflow by writing logic "where you need it and where you understand the need" (and without having to create a new extension)

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.

3 participants