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

Avoid singletons check when gc cleanup #440

Conversation

piiertho
Copy link
Member

@piiertho piiertho commented Mar 5, 2023

Avoid registration of singletons when those are sent from cpp first.
Set ObjectId when constructing singletons.

@piiertho piiertho added bug Something isn't working GD4 labels Mar 5, 2023
@piiertho piiertho requested review from chippmann and CedNaru March 5, 2023 18:25
Copy link
Member

@CedNaru CedNaru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I do not like the approach here.
I don't want to transform singletons into an exception in the regular initialization code of the KtObjects.
Something I would do is to create a function initializing all singletons on the JVM side ,that would be called when our module starts, just like we do with the rest of the API when constructors and functions index are set.

@piiertho piiertho force-pushed the bugfix/avoid-singletons-gc-registration-when-sent-from-cpp-first branch from 3bc47c5 to c423060 Compare March 7, 2023 10:03
@piiertho piiertho requested a review from CedNaru March 7, 2023 10:41
@piiertho piiertho force-pushed the bugfix/avoid-singletons-gc-registration-when-sent-from-cpp-first branch from c423060 to 96338a1 Compare March 8, 2023 14:52
@piiertho piiertho changed the title Avoid singletons gc registration when sent from cpp first Avoid singletons check when gc cleanup Mar 8, 2023
@piiertho piiertho merged commit 801f539 into 4.0-adaptation Mar 9, 2023
@piiertho piiertho deleted the bugfix/avoid-singletons-gc-registration-when-sent-from-cpp-first branch March 9, 2023 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants