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

Library may break if JS layer is reloaded by RN while the server is active #56

Closed
dagouzhi opened this issue Jul 15, 2023 · 9 comments
Closed
Labels
P2 Important issue. Ready Ready for release.

Comments

@dagouzhi
Copy link

If you call codePush.restartapp(); An error occurs when server.start() is called

another server instance is active

react-native-code-push@8.0.2
@dr.pogodin/react-native-static-server@0.8.0

@birdofpreyru birdofpreyru added the P2 Important issue. label Jul 15, 2023
@birdofpreyru
Copy link
Owner

Hey @dagouzhi , technically what happens here, I believe, codePush.restartapp() reloads JS layer of the library, it does not stop the actual server in the Native layer, thus the error when reloaded JS layer attempts to start a fresh server, without killing the old one first.

A quick workaround for you — be sure to call server.stop() prior to the restart.

A proper fix will be to hook it up to automatically kill the server in native layer when such reload of JS happens. At the moment, I am not sure whether it can be done in JS layer (which would be perfect), I think I know how to do within the native layer on Android, but then not sure how to do it in the native layer for other platforms. Thus, one way or another, I'll need some time to investigate and fix it. On what platform have you encountered this problem?

@dagouzhi
Copy link
Author

@birdofpreyru I could try to do this, but hopefully it would be better to have a static method that kills all services, or to call the start method with a configuration that forces it to start, even if there were previously active services

@birdofpreyru
Copy link
Owner

but hopefully it would be better to have a static method that kills all services, or to call the start method with a configuration that forces it to start, even if there were previously active services

It will work great for your scenario, but I am afraid in other scenarios it will open doors to hard-to-triage bugs. So, the best permanent solution is just to figure out all situations when the current server should be killed automatically, and do just it.

@birdofpreyru
Copy link
Owner

Let’s keep it open, I need it as a reminder to look into it.

@birdofpreyru birdofpreyru reopened this Jul 17, 2023
@birdofpreyru birdofpreyru changed the title server.start not work Library breaks if JS layer is reloaded by RN Jul 17, 2023
@birdofpreyru birdofpreyru changed the title Library breaks if JS layer is reloaded by RN Library breaks if JS layer is reloaded by RN while the server is active Jul 17, 2023
@birdofpreyru birdofpreyru changed the title Library breaks if JS layer is reloaded by RN while the server is active Library may break if JS layer is reloaded by RN while the server is active Jul 18, 2023
@birdofpreyru
Copy link
Owner

For the record — the issue I am tracking and intend to solve here is the following — when JS layer in RN is reloaded (e.g. in dev mode, or by some other setup that does such reload) the library may break with "another server instance is active" error because the atomicity of server start-up and shut-down operations is currently enforced in JS layer, and thus breaks across JS reload. In other words, when .stop() is called on a server instance just before JS reload (it still should be called explicitly, or implicitly due to the "Stop in Background" feature), and then .start() is called on a server instance just after the reload, it is now possible that in the native layer the start of new server instance happens prior the shutdown and clean-up of the previous server instance has completed.

@birdofpreyru birdofpreyru added the In Progress The issue is actively worked upon label Jul 18, 2023
@birdofpreyru birdofpreyru added Ready Ready for release. and removed In Progress The issue is actively worked upon labels Jul 21, 2023
@Quangdm-cdm
Copy link

@birdofpreyru hi. I still facing this issue. Can you re-check and give me some solution? Thank you so muchh :((

@birdofpreyru
Copy link
Owner

@Quangdm-cdm will you elaborate, please, what exactly do you do when encountering this issue? Are you stopping the server before the JS layer reload?

@Quangdm-cdm
Copy link

@Quangdm-cdm will you elaborate, please, what exactly do you do when encountering this issue? Are you stopping the server before the JS layer reload?

Yes i do, i also log server state before start server and result is INACTIVE. But it still show error another server instance is active

@birdofpreyru
Copy link
Owner

@Quangdm-cdm try to log the result of await getActiveServerId() — that's the only method in the library that actually makes an async call to the native side to check whether there is an active server or not; alternative ways to get server state just return the state recorded on JS side, which may be set to INACTIVE as soon as a request to stop is sent to the native side, but without waiting the request is actually fulfilled; and if you reset the JS layer entirely, then JS side just assumes there is no active server on the native side.

Do you wait for stop() promise to resolve?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Important issue. Ready Ready for release.
Projects
None yet
Development

No branches or pull requests

3 participants