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

Recycle JXcore on a regular basis #448

Open
yaronyg opened this Issue Jan 14, 2016 · 2 comments

Comments

Projects
None yet
2 participants
@yaronyg
Member

yaronyg commented Jan 14, 2016

Running a server constantly for a long time is an invitation to memory leaks and general instability. So ideally we would just shut down JXcore completely every once in a while in order to let it start up fresh. This would require putting some kind of native wrapper to decide when to do this, to make sure we are in the background when it happens and to then restart.

@yaronyg yaronyg added the Icebox label Feb 9, 2016

@mohlsen

This comment has been minimized.

Show comment
Hide comment
@mohlsen

mohlsen Apr 1, 2016

Member

this might be a workaround for jxcore/jxcore-cordova#133 / #449. If we can restart, then when we resume from background, we could restart, and not have these issues.

Member

mohlsen commented Apr 1, 2016

this might be a workaround for jxcore/jxcore-cordova#133 / #449. If we can restart, then when we resume from background, we could restart, and not have these issues.

@yaronyg

This comment has been minimized.

Show comment
Hide comment
@yaronyg

yaronyg Apr 4, 2016

Member

@mohlsen This bug requires a full recycling of all JXcore, not just a sub-thread or sub-process. The reason being that the parent process can itself become unstable and so needs to be recycled. Nobody has figured out how to do this and Oguz wasn't suggesting it in jxcore/jxcore-cordova#133.

The issue Oguz was talking about in jxcore/jxcore-cordova#133 is that at one point apparently the ONLY way to deal with memory corruptions in iOS's network stack was to nuke JXcore. That could be handled via the sub-thread approach since the network sockets belong to the thread/process. The cost though of this approach is that we end up with two full instances of Spider Monkey and that a'int cheap. But thankfully the issue was fixed and now all we should have to do is just to make sure to shut down all the incoming and outgoing sockets before we go into background. We can keep using the existing JXcore instance when we come back.

The second bug has to do with an issue that we just ran out of time. We didn't have any time to test the behaviors needed to make sure we were acting correctly when iOS put us into the background for a few minutes before killing us. So we cut that feature. But being able to nuke JXcore won't fix the problems there. The problems were that we just needed to write dedicated tests to make sure we weren't doing anything bad and we needed to add a new event to tell us that we are going into the background but aren't quite dead yet.

Member

yaronyg commented Apr 4, 2016

@mohlsen This bug requires a full recycling of all JXcore, not just a sub-thread or sub-process. The reason being that the parent process can itself become unstable and so needs to be recycled. Nobody has figured out how to do this and Oguz wasn't suggesting it in jxcore/jxcore-cordova#133.

The issue Oguz was talking about in jxcore/jxcore-cordova#133 is that at one point apparently the ONLY way to deal with memory corruptions in iOS's network stack was to nuke JXcore. That could be handled via the sub-thread approach since the network sockets belong to the thread/process. The cost though of this approach is that we end up with two full instances of Spider Monkey and that a'int cheap. But thankfully the issue was fixed and now all we should have to do is just to make sure to shut down all the incoming and outgoing sockets before we go into background. We can keep using the existing JXcore instance when we come back.

The second bug has to do with an issue that we just ran out of time. We didn't have any time to test the behaviors needed to make sure we were acting correctly when iOS put us into the background for a few minutes before killing us. So we cut that feature. But being able to nuke JXcore won't fix the problems there. The problems were that we just needed to write dedicated tests to make sure we weren't doing anything bad and we needed to add a new event to tell us that we are going into the background but aren't quite dead yet.

@yaronyg yaronyg added the Android label Sep 1, 2016

@yaronyg yaronyg added enhancement iOS and removed 0 - Icebox labels Oct 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment