rust-objc-foundation
rust-objc-foundation copied to clipboard
memory leak in INSstring.as_str()
how to reproduce
pub fn leak() {
let nsstrig = NSString::from_str("aaabbb");
nsstrig.as_str();
}
repo with examples and tests https://github.com/Lurk/nsstring_leak
why it is leaking
INSString.as_str() internaly uses UTF8String property of NSString. Apple doc says that the memory behind this pointer has a lifetime shorter than a lifetime of an NSString itself. But apparently, this is not entirely true. At least, this statement is not valid for strings that contain characters outside the ASCI range. And sometimes for strings that do not.
Sadly, I did not find any reason for that.
So, in the end, the actual leak occurs not in INSString.as_str() but, I guess, in objc runtime.
Is there a workaround?
Yes. NSString::getCString (Apple doc) and we can use it like this:
pub fn nsstring_to_rust_string(nsstring: Id<NSString>) -> String {
unsafe {
let string_size: usize = msg_send![nsstring, lengthOfBytesUsingEncoding: 4];
// + 1 is because getCString returns null terminated string
let buffer = libc::malloc(string_size + 1) as *mut c_char;
let is_success: bool = msg_send![nsstring, getCString:buffer maxLength:string_size+1 encoding:4];
if is_success {
// CString will take care of memory from now on
CString::from_raw(buffer).to_str().unwrap().to_owned()
} else {
// In case getCString failed there is no point in creating CString
// So we must free memory
libc::free(buffer as *mut c_void);
// Original NSString::as_str() swallows all the errors.
// Not sure if that is the correct approach, but we also don`t have errors here.
"".to_string()
}
}
}
In the repo I mentioned above, you will find a test and benchmark for that.
The only problem I see with this solution is that it has a different return type (String instead of &str). If you know how to fix that, or any other idea on how to do things better - please let me know.
Found a way to get rid of libc::malloc and libc::free
pub fn convert_with_vec(nsstring: Id<NSString>) -> String {
let string_size: usize = unsafe { msg_send![nsstring, lengthOfBytesUsingEncoding: 4] };
let mut buffer: Vec<u8> = vec![0_u8; string_size + 1];
let is_success: bool = unsafe {
msg_send![nsstring, getCString:buffer.as_mut_ptr() maxLength:string_size+1 encoding:4]
};
if is_success {
// before from_vec_with_nul can be used https://github.com/rust-lang/rust/pull/89292
// nul termination from the buffer should be removed by hands
buffer.pop();
unsafe {
CString::from_vec_unchecked(buffer)
.to_str()
.unwrap()
.to_string()
}
} else {
// In case getCString failed there is no point in creating CString
// Original NSString::as_str() swallows all the errors.
// Not sure if that is the correct approach, but we also don`t have errors here.
"".to_string()
}
}
updated repo with examples and benchmarks https://github.com/Lurk/nsstring_leak
Pretty sure you can fix the memory leak with:
use objc::rc::autoreleasepool;
use objc_foundation::{NSString, INSString};
autoreleasepool(|| {
let s = NSString::from_str("aaabbb");
s.as_str();
});
See my explanation here: https://github.com/SSheldon/rust-objc/pull/103#issuecomment-894464665 (this pull request would also be the way to fix this problem, because we would change the signature of NSString::as_str
to require an AutoreleasePool
reference).
@madsmtm thank you, with autoreleasepool leak is gone.
Do you have any idea why getCString might be slightly faster than UTF8String? My benchmark shows those numbers:
to string/old time: [12.551 us 12.644 us 12.733 us]
to string/autorelease time: [12.775 us 12.998 us 13.207 us]
to string/vec time: [10.107 us 10.208 us 10.307 us]
Haven't looked much at the performance of these things, but a quick guess would be that UTF8String
has to do more work; it would at the very least have:
- An extra check for whether it has to allocate or not
- Some code to autorelease the allocated buffer But at these speeds (and when we don't have the source code for it) it's hard to tell.
Oh, and btw, and just plugging 4
in as the encoding is UB, integers are i32
by default while the expected parameter is NSStringEncoding
(typedef of NSUInteger
, roughly usize
). You should define a variable/constant with an explicit type or use 4usize
, otherwise msg_send!
will infer the wrong type!