search

touchlab / SQLiter

Minimal multiplatform sqlite library
https://touchlab.co
173 stars 35 forks source link

Memory leak fixes #106

Closed hbmartin closed 10 months ago

hbmartin commented 10 months ago

This PR is the result of a memory leak investigation started in https://github.com/apollographql/apollo-kotlin/issues/5262 The cause seems to be the KN GC incorrectly not releasing strings in a CFString -> NSString -> (Kotlin) String allocation This PR attempts to fix this in 3 ways:

  1. Allocate a single empty string constant as a fallback for sqlite3_column_text instead of allocating a new empty string on each call.
  2. Wrap the bytesToString call for appleMain in an autoreleasepool block.
  3. Upgrading to Kotlin 1.9.20 to take advantage of the custom allocator.

The Kotlin upgrade entailed other work:

  1. Removing support for mingwX86
  2. Configuring additional opt-in flags
  3. Disabling the new default hierarchy
  4. Migrating AtomicInt references

Look forward to feedback and discussion here!

kpgalligan commented 10 months ago

Assume the autorelease is cleaning up the NSString returned from NSString.create. That's a surprise. Will take a deeper look tomorrow/Tuesday and push out a build.

hbmartin commented 10 months ago

I did not anticipate fighting with junit to be the hard part of this... still trying to solve

hbmartin commented 10 months ago

I took a stab at reading the binary test outputs https://gist.github.com/hbmartin/a2af9046f0d05c1b6a6d64267136d933 It seems that recording the entire sqlite logging output during tests was causing some overflows in those outputs. My fix was to create a NoneLogger and use that for all test cases. 🤷

I also noticed that a couple of tests were calling co.touchlab.sqliter.internal.File.exists property as a function and fixed that. Seems odd that this ever worked - my wild guess is that the complier may have allowed this since the actuals were implemented as backing properties.