Fix: Update to the latest version of SQLite3 to fix memory leaks

[HakonHarnes]

The version of SQLite3 currently used by Sioyek suffers from a memory leak in the sqlite3MemRealloc function. This PR updates the SQLite3 library to its latest version to address this issue.

Valgrind output before the update:

==334845== LEAK SUMMARY:
==334845==    definitely lost: 250,896 bytes in 19 blocks
==334845==    indirectly lost: 510,123 bytes in 1,523 blocks
==334845==      possibly lost: 63,264 bytes in 25 blocks
==334845==    still reachable: 10,908,591 bytes in 5,531 blocks
==334845==                       of which reachable via heuristic:
==334845==                         length64           : 272 bytes in 4 blocks
==334845==         suppressed: 0 bytes in 0 blocks
==334845==
==334845== ERROR SUMMARY: 3469 errors from 65 contexts (suppressed: 0 from 0)

Valgrind output after the update:

==357665== LEAK SUMMARY:
==357665==    definitely lost: 251,128 bytes in 19 blocks
==357665==    indirectly lost: 565,220 bytes in 2,538 blocks
==357665==      possibly lost: 60,376 bytes in 1 blocks
==357665==    still reachable: 10,856,467 bytes in 4,548 blocks
==357665==                       of which reachable via heuristic:
==357665==                         length64           : 272 bytes in 4 blocks
==357665==         suppressed: 0 bytes in 0 blocks
==357665==
==357665== ERROR SUMMARY: 2974 errors from 44 contexts (suppressed: 0 from 0)

Related issue: #693 (This issue can't be closed after merging since there are still memory leaks)

[ahrm]

Shouldn't this be fixed in the upstream sqlite repository? I am hesitant to accept this because I assume the sqlite code is solid and if there is a memory leak is probably because I have messed up somewhere in my code.

[HakonHarnes]

Shouldn't this be fixed in the upstream sqlite repository?

I just checked, and it's already fixed in the sqlite repository:

static void *sqlite3MemRealloc(void *pPrior, int nByte){
  struct MemBlockHdr *pOldHdr;
  void *pNew;
  assert( mem.disallow==0 );
  assert( (nByte & 7)==0 );     /* EV: R-46199-30249 */
  pOldHdr = sqlite3MemsysGetHeader(pPrior);
  pNew = sqlite3MemMalloc(nByte);
  if( pNew ){
    memcpy(pNew, pPrior, (int)(nByte<pOldHdr->iSize ? nByte : pOldHdr->iSize));
    if( nByte>pOldHdr->iSize ){
      randomFill(&((char*)pNew)[pOldHdr->iSize], nByte - (int)pOldHdr->iSize);
    }
    sqlite3MemFree(pPrior);
  }
  return pNew;
}

This suggests that there is actually a memory leak. I guess the proper fix here is to just update to the latest sqlite3.c file from the upstream repo :)

[HakonHarnes]

@ahrm I've updated the original PR description.

[erolm-a]

Hello everyone.

First, I am glad someone pointed this bug out, this memory leak makes my poor laptop unresponsive if I keep sioyek open for days. I like sioyek but this can easily become a show-stopper.

Secondly, while this point goes a bit beyond the scope of this PR, I think embedding a huge library shipped into 3 single files that - for all @ahrm knows - could sneak vulnerabilities without getting noticed in a repo is not a great idea. Furthermore, I think it goes against packagers' intentions as you are introducing a dependency that is outside of the package manager's management.

Wouldn't it be better to change the way sioyek imports sqlite3, e.g. either by adding it as a submodule, as it is already done for mupdf, or (probably better) simply expecting it to be installed and accessible from qmake and asking the user to make them discoverable by qmake?

My two cents.

[HakonHarnes]

simply expecting it to be installed and accessible from qmake and asking the user to make them discoverable by qmake?

This would be the best solution (it's also the solution Zathura has opted for).

[KorigamiK]

I second this, we should have a submodule instead and the user should also be allowed to opt for strategic or dynamic linking of sqlite as well.