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

iOS and Server Error Fix Plus Potential Major Speed Increase #11

Closed
wants to merge 6 commits into from

Conversation

corepuncher
Copy link

@corepuncher corepuncher commented Sep 19, 2024

Background

A while back, I noticed that my tile server periodically would accumulate "CLOSE_WAIT" states, resulting in a gradual slowing and eventual need for a reboot. Upon researching this topic, the overarching theme was that the client was not closing connections properly, leaving the server in a confused state:

"CLOSE_WAIT - Indicates that the server has received the first FIN signal from the client and the connection is in the process of being closed. This means the socket is waiting for the application to execute close() . A socket can be in CLOSE_WAIT state indefinitely until the application closes it."

The issue was rare enough I made a monitoring script on my server that would auto-reboot if CLOSE_WAIT count got too high.
I then turned to development on iOS as Android was mostly done.

iOS Restart Issue

My iOS app would run great on first install. But then if I killed it and restarted, when it came up, it would have many missing tiles and just slow in general. Common debugger errors (hundreds) included:

Error: SocketException: Connection failed (OS Error: Too many open files, errno = 24), 
SocketException: Failed host lookup: 'site.com' (OS Error: nodename nor servname provided, or not known, errno = 8),

Sometimes I even got missing asset errors. This suggested file descriptor exhaustion.

Strangely, these errors only occurred on a physical iPhone. iOS simulators worked great, so did all my Android devices (aside from periodic CLOSE_WAIT issues seen on my server). The fix was to restart the app about 4 times, which must have cleared out "something" from memory. App would then work great, until next time. Rinse & repeat.

Initial Debugging

I decided to play around with local pub dev copy of tile_provider.dart and image_provider.dart, focusing on the closing of the connections. tile_provider.dart already had this:

  @override
  Future<void> dispose() async {
    if (_tilesInProgress.isNotEmpty) {
      await Future.wait(_tilesInProgress.values.map((c) => c.future));
    }
    _dioClient.close();
    super.dispose();
  }

but perhaps not all connections were being closed properly. I added a print statement, and noticed upon normal map usage, hundreds of prints, suggesting frequent creation and attempted closing of dio clients.

I had previously stumbled upon a Dart thread with similar errors, and they suggested using cupertino_http, and/or native_dio_adapter which includes that along with cronet for Android. Clearly dio was not playing nice with iPhones, so I gave it a shot.

I also hypothesized that maybe only a single dio client was actually needed, instead of creating and destroying constantly. That way, previously used resources could be reused.

Result

Success! Not only was I able to restart my iPhone app as frequently as I wanted with zero errors, but the tiles are FLYING IN.
At least for my app, it's night and day. Sometimes I can't even tell it's tiled, more like a seamless map. And when I zoom out quickly, instead of checker-board loading, they all load nearly simultaneously.

Who May Benefit (Most) & Testing

While more testing is needed, clearly anyone with the same errors I was experiencing on iPhones should test these changes.
After two days of usage, I have seen no ill effects. On the contrary.

It should be noted that in my application, I use MANY TIleLayer() at once (12+). Probably a somewhat rare amount, but it should work all the same.

Very few lines of code were needed as you can see. I import native_dio_adapter, created a singleton of dio, and removed the _dioClient.close(). In addition, I create a CancellableNetworkTileProvider instance once in initState and use _tileProvider in all of my tile layers:

_tileProvider = CancellableNetworkTileProvider();

TileLayer(
  tileProvider: _tileProvider,
  ...
),

It seems clear to me that at least for iOS, sometimes dio instances were not all getting closed properly. But this must have also been true for Android, just to a lesser extent, given my experience with CLOSE_WAIT on my tile server. Installing the native adapters fixed the file descriptors running out error on iPhone, and I'm guessing I will no longer get the server errors.

Errors aside, I am guessing that the more TileLayer() you have in your app, the more of a speed increase you will see.

I am eager to hear if this works for you!

PS: if this works well for everyone, you might also consider the same for NetworkTileProvider, which I do not currently use.

Created file for dio singleton.  Uses NativeAdapter() if !kIsWeb.
Uses single instance of Dio with NativeAdapter.
Use Dio singleton.  No longer a need to close in dispose.
@JaffaKetchup
Copy link
Member

Hey @corepuncher,
I've just had a chance to look through this.
There are some issues with code and style, but that's no problem, I'm happy to go through and fix those (although I'm surprised it compiles for you).
The biggest problem is that using cupertino_http seems as though it might break the cancellable part, meaning it effectively doesn't fit in this package anymore.
Is there any reason you can't pass the Cupertino HTTP client through to the normal NetworkTileProvider? You may need to fork flutter_map to remove the _httpClient.dispose() line again.

@corepuncher
Copy link
Author

Oops rookie mistake with PR.

FYI, as a test, I replaced Cancellable with the default Network in my app, just to see if it was dio / cancellable aspect that was causing all the issues.

Result: Poor performance returned. Worked at first, then had major issues getting files to download thereafter. So probably the cupertino http is needed either way for Platform.isIOS.

But do not forget....I was having "CLOSE_WAIT" issues on my server before I even started coding iOS (only Android), which may indicate some manner of file descriptor mismanagement at some level. Therefore, it may also be wise to use, or have the option to use, cronet (included in native dio http), which basically just means implement the native stuff all around regardless. Hopefully the cancellable aspect will still work.

A short term solution could also be to add another property/flag "Use native HTTP" or something like that, then users like myself could turn that on.

I hope I have at least helped a little with my report, but I do think it would be best for someone to take this over at this point.

I do not have time to delve in properly right now, and you know your code best. I was lucky enough to fix the root issue for me for now, but have to turn to other issues at present.

I left this fork editable, so please feel free to have your way with it. My app is not pointed to it directly.

- NativeAdapter  (cupertino http) required for iOS.
- Cronet may or may not work with Android. There are problems when too many parallel requests. As such, only iOS platforms are currently using the native package.
dioClient is passed in from tile_provider.dart
Passes dioClient to ImageProvider
@corepuncher corepuncher closed this by deleting the head repository Oct 5, 2024
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.

2 participants