search

dart-lang / native

Dart packages related to FFI and native assets bundling.
BSD 3-Clause "New" or "Revised" License
82 stars 27 forks source link

`NSData.dataWithBytesNoCopy_length_` is crashing #1107

Open liamappelbe opened 2 weeks ago

liamappelbe commented 2 weeks ago

ns_data_test.dart causes some sort of seg faults crash loop when using NSData.dataWithBytesNoCopy_length_. I'm switching to NSData.dataWithBytes_length_ for now, which fixes the crash but is slower. Logs.

I noticed the crash in https://github.com/dart-lang/native/pull/1100, but it predates that PR (that log is from a nightly run). I haven't been able to get GDB working on my macbook, so I'm not sure exactly where the crash is happening. From the logs it seems to be happening when the NSData is destroyed.

brianquinlan commented 2 weeks ago

@dcharkes

NSData dataWithBytesNoCopy:length:freeWhenDone: takes ownership of a block of memory that was allocated with malloc.

Is the package:ffi malloc the (presumably) libc malloc that NSData expects?

dcharkes commented 2 weeks ago

I haven't been able to get GDB working on my macbook

LLDB is the Mac variant.

Is the package:ffi malloc the (presumably) libc malloc that NSData expects?

https://github.com/dart-lang/native/blob/6a9282c7e2abd5d5ad89a2de1eb985ee9fc63262/pkgs/ffi/lib/src/allocation.dart#L15-L16

Yep, it's malloc in the current process.

brianquinlan commented 2 weeks ago

I haven't been able to get GDB working on my macbook

LLDB is the Mac variant.

Is the package:ffi malloc the (presumably) libc malloc that NSData expects?

https://github.com/dart-lang/native/blob/6a9282c7e2abd5d5ad89a2de1eb985ee9fc63262/pkgs/ffi/lib/src/allocation.dart#L15-L16

Yep, it's malloc in the current process.

I'm not sure how this fits together. Presumably the malloc of the current process could mean tcmalloc or jemalloc, depending on how the application is linked? But I'd guess that NSData uses whatever the standard deallocator is? Or maybe not ;-)

Apple's documentation isn't very clear on this. I also notice that we don't use NSData dataWithBytesNoCopy:length:freeWhenDone: at Google at all.

I think that this code is probably OK how it is now. If people complain about the performance when using large blocks of memory, we could use initWithBytesNoCopy:length:deallocator: