[Core] Call wait_for_alert in a thread

[DjLegolas]

This spawns a thread in alertmanager to call wait_for_alert in a thread. This reduces latency to deluge responding to events.

Based on #175.

[DjLegolas]

I think everything is fixed.

[aresch]

This pull request looks good to me. It would be nice to have some better tests for alert manager, as we don't cover much of what has changed. Feel free to follow-up with some expanded tests if you're interested :)

[DjLegolas]

Hmm, ok, I'll give it a try. Correct me if I'm wrong, but to fully test AlertManager, I'll need to make libtorrent post some alerts and to handle them correctly. One way is to mock it, and the other is to somehow force is to set an alert. Which is the better way?

[cas--]

Considering the time it takes alerts to be posted, sometimes ~0.5s, mocking libtorrent is a better route. Although a lot of tests rely on libtorrent, it's not the best way of unit testing... :)

[aresch]

Hmm, ok, I'll give it a try. Correct me if I'm wrong, but to fully test AlertManager, I'll need to make libtorrent post some alerts and to handle them correctly. One way is to mock it, and the other is to somehow force is to set an alert. Which is the better way?

Yea, you'll want to mock out session.pop_alerts() and session.wait_for_alerts(). You can check out how we did it in spritzle for an idea: https://github.com/spritzle/spritzle/blob/master/spritzle/tests/test_alert.py

[DjLegolas]

So I wrote some test units... What do you guys think @aresch @cas--?

[DjLegolas]

Any thoughts why this change cases tox -e py3 to fail? I tested it with python 3.5, 3.6, 3.7 and only with 3.7 all tests pass.

[DjLegolas]

Rebased and fixed last 2 changes requested by @aresch

[cas--]

@DjLegolas I imagine that the segfault in tests is due to https://github.com/arvidn/libtorrent/issues/6437

[DjLegolas]

Saving the segfault job for later use. @cas-- how do you want to handle this? We could try to perform a deep-copy of the alerts

[cas--]

I am not sure this is stable, I just had it crash on me as soon as I tested the branch out

I am not willing to merge this while there is the potential for segfaults in alert handling.

[cas--]

@DjLegolas I have merged the changes for component 0745c0eff85ea4e4a3646c454098503ed08c56ff and 25a2b113e290600043338e22b4f116cd2dbf2886 but I am finding that the alertmanager thread changes is increasing the propagation of segfaults due to the alert pointer going out of scope... I will investigate further on how we can workaround the issue.

[cas--]

Thanks @DjLegolas

I applied a refactor to remove the alert copy and avoid segfaults: https://github.com/deluge-torrent/deluge/commit/54d6f502319ea60de6cbe34f1c22e7eed367f1ee

I'm not sure what effect this will have for alert handling so might need to keep an eye on it and revisit if needed.