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

Possible memory leak #38

Closed
mcrakhman opened this issue Mar 23, 2016 · 4 comments
Closed

Possible memory leak #38

mcrakhman opened this issue Mar 23, 2016 · 4 comments
Labels

Comments

@mcrakhman
Copy link

Hi!

I used you library to format phone numbers entered by user. And though I liked PhoneNumberKit a lot, I discovered certain memory leaks.

Please note that I updated the library to version 0.6.4 via cocoapods, just so you know that I am using the up-to-date version.

It seems both of the following lines cause memory leaks (thus if I delete both of them the leaks go away). Also PhoneNumber (rawNumber:) was tested in different apps with the same leaks.

do {
       _ = try PhoneNumber (rawNumber: phone!)
}
catch {
      isValidNumber = false
}

And this one:

let formattedString = PartialFormatter ().formatPartial (cutString)

cutString is a string starting with + and ending with numbers.

Below see instruments' screenshots.

instruments - 1

instruments - 2

@marmelroy
Copy link
Owner

Thanks @mcrakhman. Looks suspicious. I will investigate further this evening.

@marmelroy marmelroy added the bug label Mar 24, 2016
@marmelroy
Copy link
Owner

After some investigation - PhoneNumberKit keeps the parsed metadata in memory and a memory leak can, under certain circumstances, happen.

I've begun work on a significant change to PhoneNumberKit that will be a lot more explicit about ownership and will resolve these issues. It will also introduce some breaking changes to the API.

That work is on the https://github.com/marmelroy/PhoneNumberKit/tree/dev branch.

@marmelroy
Copy link
Owner

Good news!

I was able to resolve the memory leak issues without an API change. Bundling all the changes in release 0.7.

Thanks again for reporting these @mcrakhman.

@mcrakhman
Copy link
Author

Thank you very much! Will update and try.

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

No branches or pull requests

2 participants