From 6370e358b5a32c8866be28e2dd1cc80f2be37e8f Mon Sep 17 00:00:00 2001 From: Robin Watts Date: Wed, 21 Jun 2017 18:14:02 +0100 Subject: Harfbuzz tweaks. Avoid defining any functions/variables beginning with hb_ to avoid potential namespace clashes. Clarify language about why the Harfbuzz workarounds are needed. --- source/fitz/harfbuzz.c | 84 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 60 insertions(+), 24 deletions(-) (limited to 'source/fitz') diff --git a/source/fitz/harfbuzz.c b/source/fitz/harfbuzz.c index 80451278..be70cdd6 100644 --- a/source/fitz/harfbuzz.c +++ b/source/fitz/harfbuzz.c @@ -9,14 +9,31 @@ #include -/* Harfbuzz has some major design flaws. +/* Harfbuzz has some major design flaws (for our usage + * at least). * - * It is utterly thread unsafe. It uses statics to hold - * structures in, meaning that if it is ever called from - * more than one thread at a time, access to such - * structures can race. We work around this by imposing - * a lock on our calls to harfbuzz (we reuse the freetype - * lock). + * By default it uses malloc and free as the underlying + * allocators. Thus in its default form we cannot get + * a record (much less control) over how much allocation + * is done. + * + * Harfbuzz does allow build options to control where + * malloc and free go - in particular we point them at + * fz_hb_malloc and fz_hb_free in our implementation. + * Unfortunately, this has problems too. + * + * Firstly, there is no mechanism for getting a context + * through the call. Most other libraries allow us to + * pass a "void *" value in, and have it passed through + * to arrive unchanged at the allocator functions. + * + * Without this rudimentary functionality, we are forced + * to serialise all access to Harfbuzz. + * + * By taking a mutex around all calls to Harfbuzz, we + * can use a static of our own to get a fz_context safely + * through to the allocators. This obviously costs us + * performance in the multi-threaded case. * * This does not protect us against the possibility of * other people calling harfbuzz; for instance, if we @@ -30,9 +47,9 @@ * * In order to ensure that allocations throughout mupdf * are done consistently, we get harfbuzz to call our - * own hb_malloc/realloc/calloc/free functions that + * own fz_hb_malloc/realloc/calloc/free functions that * call down to fz_malloc/realloc/calloc/free. These - * require context variables, so we get our hb_lock + * require context variables, so we get our fz_hb_lock * and unlock to set these. Any attempt to call through * without setting these will be detected. * @@ -40,12 +57,31 @@ * handlers are shared between all the fz_contexts in * use at a time. * - * Finally, harfbuzz stores various malloced structures - * in statics. These can either be left to leak on - * closedown, or (if HAVE_ATEXIT is defined), they will - * be freed using various 'atexit' callbacks. Unfortunately - * this won't reset the fz_context value, so the allocators - * can't free them correctly. + * Secondly, Harfbuzz allocates some 'internal' memory + * on the first call, and leaves this linked from static + * variables. By default, this data is never freed back. + * This means it is impossible to clear the library back + * to a default state. Memory debugging will always show + * Harfbuzz as having leaked a set amount of memory. + * + * There is a mechanism in Harfbuzz for freeing these + * blocks - that of building with HAVE_ATEXIT. This + * causes the blocks to be freed back on exit, but a) + * this doesn't reset the fz_context value, so we can't + * free them correctly, and b) any fz_context value it + * did keep would already have been closed down due to + * the program exit. + * + * In addition, because of these everlasting blocks, we + * cannot safely call Harfbuzz after we close down any + * allocator that Harfbuzz has been using (because + * Harfbuzz may still be holding pointers to data within + * that allocators managed space). + * + * There is nothing we can do about the leaking blocks + * except to add some hacks to our memory debugging + * library to allow it to suppress the blocks that + * harfbuzz leaks. * * Consequently, we leave them to leak, and warn Memento * about this. @@ -64,33 +100,33 @@ * problems that for now, we'll just forbid it. */ -static fz_context *hb_secret = NULL; +static fz_context *fz_hb_secret = NULL; static void set_hb_context(fz_context *ctx) { - hb_secret = ctx; + fz_hb_secret = ctx; } static fz_context *get_hb_context(void) { - return hb_secret; + return fz_hb_secret; } -void hb_lock(fz_context *ctx) +void fz_hb_lock(fz_context *ctx) { fz_lock(ctx, FZ_LOCK_FREETYPE); set_hb_context(ctx); } -void hb_unlock(fz_context *ctx) +void fz_hb_unlock(fz_context *ctx) { set_hb_context(NULL); fz_unlock(ctx, FZ_LOCK_FREETYPE); } -void *hb_malloc(size_t size) +void *fz_hb_malloc(size_t size) { fz_context *ctx = get_hb_context(); @@ -99,7 +135,7 @@ void *hb_malloc(size_t size) return fz_malloc_no_throw(ctx, size); } -void *hb_calloc(size_t n, size_t size) +void *fz_hb_calloc(size_t n, size_t size) { fz_context *ctx = get_hb_context(); @@ -108,7 +144,7 @@ void *hb_calloc(size_t n, size_t size) return fz_calloc_no_throw(ctx, n, size); } -void *hb_realloc(void *ptr, size_t size) +void *fz_hb_realloc(void *ptr, size_t size) { fz_context *ctx = get_hb_context(); @@ -117,7 +153,7 @@ void *hb_realloc(void *ptr, size_t size) return fz_resize_array_no_throw(ctx, ptr, 1, size); } -void hb_free(void *ptr) +void fz_hb_free(void *ptr) { fz_context *ctx = get_hb_context(); -- cgit v1.2.3