tdlib / td

Cross-platform library for building Telegram clients
https://core.telegram.org/tdlib
Boost Software License 1.0
7.11k stars 1.44k forks source link

waitsForConnectivity, configurations for URLSession #2501

Closed vincentneo closed 1 year ago

vincentneo commented 1 year ago

Attempts to fix a part of #2499, comment:

Here's the interesting thing that I observed today, even after re-enabling IPv4 (and at the point where IPv4 websites are already usable in Safari), the errors still keep streaming in, and attempting to open a chat (or do anything basically, that being an example), just didn't work.

I made this minor change, observation seems to suggest that this at least recovers after connection is loss, rather than just not working at all after connection loss. (It does not change any IPv6 behaviour mentioned in the same issue)

levlam commented 1 year ago

It is wrong to use global variables like this. The method http_send can be called simultaneously from multiple threads.

vincentneo commented 1 year ago

It is wrong to use global variables like this. The method http_send can be called simultaneously from multiple threads.

Please advise on what to change. Personally know nothing much about C++/Objective-C++, so yeah.

On second thought, the referencing of the same NSURLSession object from multiple threads shouldn't be a problem, if the current implementation of referencing the singleton object NSURLSession.sharedSession didn't pose a problem, isn't it?

The problem would be at the point where I initialise the NSURLSession object I guess?

levlam commented 1 year ago

Yes.

The code should look like this:

NSURLSession *get_session() {
  static NSURLSession *urlSession = [] {
    auto configuration = [NSURLSessionConfiguration defaultSessionConfiguration];
    configuration.networkServiceType = NSURLNetworkServiceTypeResponsiveData;
    configuration.waitsForConnectivity = true;
    return [NSURLSession sessionWithConfiguration:configuration];
  }();
  return urlSession;
}

This way it still leaks the urlSession, which is never freed, but it should work correctly.

levlam commented 1 year ago

Also, does it work without waitsForConnectivity? TDLib has its own time limits for requests and starts new requests when the limits exceeded. If the requests will wait for connectivity instead of failing, there could be a lot of pending unneeded requests.

vincentneo commented 1 year ago

Also, does it work without waitsForConnectivity?

NSURLSession.sharedSession has waitsForConnectivity set to false. So with the build of the master branch, latest commit, the connection if at any point breaks, it appears it will never regain the connection.

(example: Turn network off, open app, turn network on, send a message from another phone, proceeds to not receive it on the watch client)

As for NSURLNetworkServiceTypeResponsiveData it's just a QoS optimisation thing. Apple docs suggests to use this for "situations where the user is anticipating a quick response, like instant messaging or completing a purchase."

levlam commented 1 year ago

Thank you. I will merge the pull request after the next Telegram update.