bykof / cordova-plugin-webserver

A webserver plugin for cordova
Other
120 stars 51 forks source link

Memory usage #22

Open kukukk opened 5 years ago

kukukk commented 5 years ago

Hi!

When I played with the project on iOS I encountered some memory issues. I think it was because in Webserver.swift you never empty the self.responses variable. I used the webserver to forward files having a few MB in size and it caused the high memory usage. Maybe you should do some proper cleanup and after the request is processed remove it from self.responses.

bykof commented 5 years ago

Woops! You are right... should do some corrections. Can you tell me, for what purpose you use cordova plugin webserver?

kukukk commented 5 years ago

We have a project where we use multiple servers as sources for streaming and the client combines the segments received from the different servers and feed the HTML5 player with the result using the MSE extension. Unfortunately, Apple decided not to implement MSE on iOS. As a workaround, we tried to use a local webserver as a proxy and implement the logic for combining the segments and provide the result as HTTP Live Streaming for the HTML5 player. And it was easy to use your plugin for a quick test.

I also had to make a small modification to support specifying the Content-Type instead of using hard coded text/html for this to work. Since I only need it on iOS, I don't have a general solution to make it working on all platforms.

bykof commented 5 years ago

@kukukk sick... Sounds great why other people use my plugin :D

don't have a general solution to make it working on all platforms

There is no general solution... one have to fix it in the code for android and iOS. I don't know when I could implement the fix.

kukukk commented 5 years ago

Yeah, it's kinda' sick :}, but since Apple forces every third-party browser to use the built-in WebKit rendering engine we don't really have a choice, if we want to support iOS...

I have some experiences with Android, so I will try to have a look at the Android part too. But I can't promise it.

Btw, I had a quick look at NanoHTTPDWebserver.java and it seems that this.webserver.responses is not cleaned either.

bykof commented 5 years ago

Yes, that's why I said :)

one have to fix it in the code for android and iOS

Just pull out the response from this.webserver.responses. It should do the trick on the iOS sources too

kukukk commented 5 years ago

Hi!

Unfortunately, it seems to be more complicated. When I try to remove the handled response from this.webserver.responses it make the app to crash. I tried to different ways:

When "We got the dict so put information in the response" use

let responseDict = self.responses.removeValue(forKey: requestUUID) as! Dictionary<AnyHashable, Any>

Before "Complete the async response"

if self.responses.index(forKey: requestUUID) != nil {
    self.responses.removeValue(forKey: requestUUID)
}

I also tried to just mark the response as handled by

responseDict["handled"] = true
self.responses[requestUUID] = responseDict

but it also makes the app to crash.

I think it is because I'm trying to manipulate the dictionary in different threads. Unfortunately, I have no experiences with Swift to make it thread-safe. I will try to find a solution, but, if you have some time, it would be greatly appreciated some help :}

kukukk commented 5 years ago

I tried to implement a basic thread-safe Dictionary implementation, which seems to work. You can have a look at it here: https://github.com/kukukk/cordova-plugin-webserver/commit/29ab495aadd22c46af1501cc9eb620a0bf6a43cd

bykof commented 5 years ago

@kukukk looks good, can you make a pull request to merge it in here?

kukukk commented 5 years ago

One more observation: I think NanoHTTPD is also using threads to handle the requests. It means that simply removing the handled request will also cause a crash. However, on Android it may be easier to fix the problem because it has a built-in ConcurrentHashMap. It's just an idea, I have not tested it.

boedy commented 5 years ago

What is the status of this issue? Has everything what has been discussed here been fixed or are there still some todo's left?

digaus commented 4 years ago

@boedy I currently run into this issue aswell. I am serving update files for smarthome devices. These devices all download the update from the smartphone. Some users have arround 40 devices which causes issues on android.