QR code reader is leaking memory since not using syms recycle

[daniel-falk]

We are using the ZBar library for continuous image analytics (i.e. scanning new frames 24/7) and when doing so we have found that the library is leaking memory when finding QR codes.

Problem description I have identified the issue to the recycling of zbar_symbol_t. When a symbol is discarded with the _zbar_image_scanner_recycle_syms() function, it is not free'd, but rather added to the scanners recycle[i] bucket. There are 5 different buckets depending on the size of the discarded symbol. When allocating a new symbol with _zbar_image_scanner_alloc_sym() it will reuse a symbol that is in the same or smaller sized bucket (but never one from a bigger bucket).

The issue is that the QR decoder will in the qr_code_data_list_extract_text() function always request a symbol with size 0 (_zbar_image_scanner_alloc_sym(iscn, ZBAR_QRCODE, 0);) and then reallocate the data field to the size needed.

This means that whenever we free the symbol it will be placed in one of the buckets with index 1 to 4, but we only reuse symbols from bucket 0. Therefore, every time we find a QR code we allocate new memory which is then never free'd, but instead stored in the recycle list. Memory is therefore continuously increasing until the application crashes.

Technically, this is not a memory leak since we never loose the reference to the memory, therefore I could not find it with valgrind, but it is not free'd until we clean-up the scanner. This means that the number of scans we can do before running out of memory is limited.

Solution I don't know if there is any simple fix to this since it seems like the QR decoder does not know from the start how big the buffer needs to be. We can therefore not allocate the symbols before we know that.

Our quick fix is to set RECYCLE_BUCKETS to 1, meaning that every recycled symbol will have its data field free'd and only the symbol struct will be recycled. Doing so we did however discover what seems like another bug in the _zbar_image_scanner_alloc_sym() function:

When a 0-sized symbol is requested, the first loop will exit with i=0 which makes sense (since these are in the 0:th index in the recycle array):

    for (i = 0; i < RECYCLE_BUCKETS - 1; i++)
    if (datalen <= 1 << (i * 2))
        break;

But then the next for-loop will never enter, as i > 0 is false:

    for (; i > 0; i--)
    if ((sym = iscn->recycle[i].head)) {
        STAT(sym_recycle[i]);
        break;
    }

This means that sym will still be zero, and a new symbol will be allocated:

    } else {
    sym = calloc(1, sizeof(zbar_symbol_t));
    STAT(sym_new);
    }

Our complete quick-fix for this is therefore:

From c7791b3ffdf2bc1102e05c5a3da9cbbcbf0a6395 Mon Sep 17 00:00:00 2001
From: Daniel Falk <daniel.falk.1@fixedit.ai>
Date: Sun, 6 Aug 2023 18:57:38 +0200
Subject: [PATCH] Remove recycling to fix leak

---
 zbar/img_scanner.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/zbar/img_scanner.c b/zbar/img_scanner.c
index 3e235d1..9d5d466 100644
--- a/zbar/img_scanner.c
+++ b/zbar/img_scanner.c
@@ -82,7 +82,7 @@
 #define dump_stats(...)
 #endif

-#define RECYCLE_BUCKETS 5
+#define RECYCLE_BUCKETS 1

 typedef struct recycle_bucket_s {
     int nsyms;
@@ -230,7 +230,7 @@ inline zbar_symbol_t *_zbar_image_scanner_alloc_sym(zbar_image_scanner_t *iscn,
    if (datalen <= 1 << (i * 2))
        break;

-    for (; i > 0; i--)
+    for (; i >= 0; i--)
    if ((sym = iscn->recycle[i].head)) {
        STAT(sym_recycle[i]);
        break;
-- 
2.25.1