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

Garbage collection has issues and leaks #19

Open
special opened this issue May 18, 2019 · 0 comments
Open

Garbage collection has issues and leaks #19

special opened this issue May 18, 2019 · 0 comments
Assignees
Labels
bug Something isn't working go

Comments

@special
Copy link
Contributor

special commented May 18, 2019

The garbage collection / reference counting of objects in the backend seems like it has some serious problems.

How this works (briefly)

The client can OBJECT_REF and OBJECT_DEREF an object. When the client holds a reference, it can use (OBJECT_RESET, INVOKE, etc) that object safely. The backend Connection holds a pointer to these objects, so they can be used and won't be Go GC'd. The client will REF any object it might use; this is any object referenced from a property value it knows or in a signal parameter. Its wrappers have JS ownership and the OBJECT_DEREF happens when they are collected by JS.

The backend also counts references to objects from other objects as they're serialized. This theoretically prevents objects from being removed when they've been used from other objects.

"Garbage collection" happens when an object isn't referenced by the client, the refcount from other objects is 0, and a grace period has passed to account for parameters and edge cases. Collected objects are flagged inactive and removed from Connection's object map. Nothing can have a valid reason to refer to that object, so it does not have to be possible to look up from the Connection. If that object is used again, it's reactivated.

Issues

The client reference seems solid, though maybe inefficient in a few places.

Signal parameters / non-property values are awkward. Currently they're handled by having a grace period of a few seconds before an object can be collected, and hoping that's enough for the client to ref any values it received and wants.

The backend reference counting could be entirely broken. This is probably leaking a lot of objects, and might be too complicated to fix. It would need some tests to figure out how/if this is actually going wrong..

I'm wondering if this can be simplified by relying more on the idea of client references and trying to get rid of the backend references.

A Theory

The requirement is that any object a client sees (as an object reference in any value, e.g. a property or parameter) stays alive as long as the client might use it.

On the backend, it's not important for Connection to know of all objects. It only needs to know objects that are or might be referenced by the client.

I think the tracking of object references to other objects can go away as long as this idea of "are or might be referenced by the client" is implementable. The backend Connection.objects would only hold those, not all known objects as it does now.

Any object that is serialized into a message to the client (the MarshalJSON path) must stay alive. That effectively automatically takes a reference for the client. Only the client can indicate when it's no longer in use. If that happens, I think this all works out.

The naive approach is to ref when an object reference is serialized (after which it must be sent!) and require the client to explicitly DEREF when no longer used. Actually counting doesn't seem appropriate because the client object isn't tracking serialized references. If it's a boolean reference, the race where a client is deref-ing an object as a new reference is sent would have to be handled somehow.

@special special added bug Something isn't working go labels May 18, 2019
@special special self-assigned this May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working go
Projects
None yet
Development

No branches or pull requests

1 participant