Skip to content

Memory leaks #63

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

Closed
dankinsoid opened this issue May 1, 2024 · 3 comments · Fixed by #64
Closed

Memory leaks #63

dankinsoid opened this issue May 1, 2024 · 3 comments · Fixed by #64
Milestone

Comments

@dankinsoid
Copy link
Contributor

I looked at the code and noticed strong reference cycles:

    /// Lazy initialized `QueryAPI`.
    public lazy var queryAPI: QueryAPI = {
        QueryAPI(client: self)
    }()

    /// Lazy initialized `DeleteAPI`.
    public lazy var deleteAPI: DeleteAPI = {
        DeleteAPI(client: self)
    }()

    /// Lazy initialized `InvokableScriptsAPI`.
    public lazy var invokableScriptsApi: InvokableScriptsAPI = {
        InvokableScriptsAPI(client: self)
    }()

and inside QueryAPI, DeleteAPI, InvokableScriptsAPI:

    /// Shared client.
    private let client: InfluxDBClient

So QueryAPI, DeleteAPI, InvokableScriptsAPI have a strong reference to the client and the client has strong references to them, creating a reference cycle that leads to a memory leak. None of these objects can be deinitialized.
Since none of these classes store anything mutable, I suggest using structs instead of classes here and making these properties computed:

    public var queryAPI: QueryAPI {
        QueryAPI(client: self)
    }

    public var deleteAPI: DeleteAPI {
        DeleteAPI(client: self)
    }

    public var invokableScriptsApi: InvokableScriptsAPI {
        InvokableScriptsAPI(client: self)
    }

InfluxDBClient could also be a struct, and it would be ideal if all these types conformed to the Sendable protocol.

@dankinsoid
Copy link
Contributor Author

There are many similar reference cycles in InfluxDB2API.

@dankinsoid
Copy link
Contributor Author

I suggest using struct instead of class wherever possible

@dankinsoid dankinsoid mentioned this issue May 1, 2024
6 tasks
@bednar
Copy link
Contributor

bednar commented May 2, 2024

Hi @dankinsoid,

Thank you for using our client and for taking the time to contribute to its development. I'm thrilled to see your Pull Request #64, which addresses the issue you've encountered.

I appreciate your initiative in improving the project and would be more than happy to assist you with your Pull Request. If you have any questions or need further guidance as you refine your submission, please feel free to reach out.

Looking forward to working together to enhance our client.

Best Regards

@bednar bednar added this to the 1.7.0 milestone May 14, 2024
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

Successfully merging a pull request may close this issue.

2 participants