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

Memory usage #22

Open
kukukk opened this issue Jul 27, 2018 · 11 comments
Open

Memory usage #22

kukukk opened this issue Jul 27, 2018 · 11 comments

Comments

@kukukk
Copy link
Contributor

kukukk commented Jul 27, 2018

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
Copy link
Owner

bykof commented Jul 27, 2018

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

@kukukk
Copy link
Contributor Author

kukukk commented Jul 27, 2018

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
Copy link
Owner

bykof commented Jul 27, 2018

@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
Copy link
Contributor Author

kukukk commented Jul 27, 2018

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
Copy link
Owner

bykof commented Jul 27, 2018

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
Copy link
Contributor Author

kukukk commented Aug 20, 2018

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
Copy link
Contributor Author

kukukk commented Aug 21, 2018

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
Copy link
Owner

bykof commented Sep 11, 2018

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

@kukukk
Copy link
Contributor Author

kukukk commented Sep 11, 2018

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
Copy link
Collaborator

boedy commented May 30, 2019

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
Copy link

digaus commented Mar 21, 2020

@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.

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

No branches or pull requests

4 participants