Bug 698353: Avoid having our API depend on DEBUG/NDEBUG.

Currently, our API uses static inlines for fz_lock and
fz_unlock, the definitions for which depend on whether
we build NDEBUG or not. This isn't ideal as it causes
problems when people link a release binary with a debug
lib (or vice versa).

We really want to continue to use static inlines for the
locking functions as used from MuPDF, as we hit them
hard in the keep/drop functions.

We therefore remove fz_lock/fz_unlock from the public API
entirely. Accordingly, we move the fz_lock/fz_unlock
static inlines into fitz-imp.h (an internal header),
together with the fz_keep_.../fz_drop_... functions.

We then have public fz_lock/fz_unlock functions for any
external callers to use that are free of compilications.

At the same time, to avoid another indirection, we change
from holding the locking functions as a pointer to a
struct to a struct itself.

22 files changed, 209 insertions, 192 deletions

diff --git a/Makefile b/Makefile
index 7c5ea532..114a6eb6 100644
--- a/Makefile
+++ b/Makefile
@@ -155,8 +155,9 @@ THREAD_OBJ := $(THREAD_SRC:%.c=$(OUT)/%.o)
 
 $(FITZ_OBJ) : $(FITZ_HDR) $(FITZ_SRC_HDR)
 $(PDF_OBJ) : $(FITZ_HDR) $(PDF_HDR) $(PDF_SRC_HDR)
-$(PDF_OBJ) : $(FITZ_SRC_HDR) # ugh, ugly hack for colorspace-imp.h
+$(PDF_OBJ) : $(FITZ_SRC_HDR) # ugh, ugly hack for fitz-imp.h + colorspace-imp.h
 $(XPS_OBJ) : $(FITZ_HDR) $(XPS_HDR) $(XPS_SRC_HDR)
+$(XPS_OBJ) : $(FITZ_SRC_HDR) # ugh, ugly hack for fitz-imp.h
 $(SVG_OBJ) : $(FITZ_HDR) $(SVG_HDR) $(SVG_SRC_HDR)
 $(CBZ_OBJ) : $(FITZ_HDR) $(CBZ_HDR) $(CBZ_SRC_HDR)
 $(HTML_OBJ) : $(FITZ_HDR) $(HTML_HDR) $(HTML_SRC_HDR)
diff --git a/include/mupdf/fitz/context.h b/include/mupdf/fitz/context.h
index 129fb686..5a88433f 100644
--- a/include/mupdf/fitz/context.h
+++ b/include/mupdf/fitz/context.h
@@ -108,11 +108,46 @@ enum
 */
 void fz_flush_warnings(fz_context *ctx);
 
+/*
+	Locking functions
+
+	MuPDF is kept deliberately free of any knowledge of particular
+	threading systems. As such, in order for safe multi-threaded
+	operation, we rely on callbacks to client provided functions.
+
+	A client is expected to provide FZ_LOCK_MAX number of mutexes,
+	and a function to lock/unlock each of them. These may be
+	recursive mutexes, but do not have to be.
+
+	If a client does not intend to use multiple threads, then it
+	may pass NULL instead of a lock structure.
+
+	In order to avoid deadlocks, we have one simple rule
+	internally as to how we use locks: We can never take lock n
+	when we already hold any lock i, where 0 <= i <= n. In order
+	to verify this, we have some debugging code, that can be
+	enabled by defining FITZ_DEBUG_LOCKING.
+*/
+
+struct fz_locks_context_s
+{
+	void *user;
+	void (*lock)(void *user, int lock);
+	void (*unlock)(void *user, int lock);
+};
+
+enum {
+	FZ_LOCK_ALLOC = 0,
+	FZ_LOCK_FREETYPE,
+	FZ_LOCK_GLYPHCACHE,
+	FZ_LOCK_MAX
+};
+
 struct fz_context_s
 {
 	void *user;
 	const fz_alloc_context *alloc;
-	const fz_locks_context *locks;
+	fz_locks_context locks;
 	fz_id_context *id;
 	fz_error_context *error;
 	fz_warn_context *warn;
@@ -354,41 +389,6 @@ int fz_use_document_css(fz_context *ctx);
 void fz_set_use_document_css(fz_context *ctx, int use);
 
 /*
-	Locking functions
-
-	MuPDF is kept deliberately free of any knowledge of particular
-	threading systems. As such, in order for safe multi-threaded
-	operation, we rely on callbacks to client provided functions.
-
-	A client is expected to provide FZ_LOCK_MAX number of mutexes,
-	and a function to lock/unlock each of them. These may be
-	recursive mutexes, but do not have to be.
-
-	If a client does not intend to use multiple threads, then it
-	may pass NULL instead of a lock structure.
-
-	In order to avoid deadlocks, we have one simple rule
-	internally as to how we use locks: We can never take lock n
-	when we already hold any lock i, where 0 <= i <= n. In order
-	to verify this, we have some debugging code, that can be
-	enabled by defining FITZ_DEBUG_LOCKING.
-*/
-
-struct fz_locks_context_s
-{
-	void *user;
-	void (*lock)(void *user, int lock);
-	void (*unlock)(void *user, int lock);
-};
-
-enum {
-	FZ_LOCK_ALLOC = 0,
-	FZ_LOCK_FREETYPE,
-	FZ_LOCK_GLYPHCACHE,
-	FZ_LOCK_MAX
-};
-
-/*
 	Memory Allocation and Scavenging:
 
 	All calls to MuPDF's allocator functions pass through to the
@@ -563,152 +563,4 @@ extern fz_alloc_context fz_alloc_default;
 /* Default locks */
 extern fz_locks_context fz_locks_default;
 
-#if defined(MEMENTO) || !defined(NDEBUG)
-#define FITZ_DEBUG_LOCKING
-#endif
-
-#ifdef FITZ_DEBUG_LOCKING
-
-void fz_assert_lock_held(fz_context *ctx, int lock);
-void fz_assert_lock_not_held(fz_context *ctx, int lock);
-void fz_lock_debug_lock(fz_context *ctx, int lock);
-void fz_lock_debug_unlock(fz_context *ctx, int lock);
-
-#else
-
-#define fz_assert_lock_held(A,B) do { } while (0)
-#define fz_assert_lock_not_held(A,B) do { } while (0)
-#define fz_lock_debug_lock(A,B) do { } while (0)
-#define fz_lock_debug_unlock(A,B) do { } while (0)
-
-#endif /* !FITZ_DEBUG_LOCKING */
-
-static inline void
-fz_lock(fz_context *ctx, int lock)
-{
-	fz_lock_debug_lock(ctx, lock);
-	ctx->locks->lock(ctx->locks->user, lock);
-}
-
-static inline void
-fz_unlock(fz_context *ctx, int lock)
-{
-	fz_lock_debug_unlock(ctx, lock);
-	ctx->locks->unlock(ctx->locks->user, lock);
-}
-
-static inline void *
-fz_keep_imp(fz_context *ctx, void *p, int *refs)
-{
-	if (p)
-	{
-		(void)Memento_checkIntPointerOrNull(refs);
-		fz_lock(ctx, FZ_LOCK_ALLOC);
-		if (*refs > 0)
-		{
-			(void)Memento_takeRef(p);
-			++*refs;
-		}
-		fz_unlock(ctx, FZ_LOCK_ALLOC);
-	}
-	return p;
-}
-
-static inline void *
-fz_keep_imp8(fz_context *ctx, void *p, int8_t *refs)
-{
-	if (p)
-	{
-		(void)Memento_checkBytePointerOrNull(refs);
-		fz_lock(ctx, FZ_LOCK_ALLOC);
-		if (*refs > 0)
-		{
-			(void)Memento_takeRef(p);
-			++*refs;
-		}
-		fz_unlock(ctx, FZ_LOCK_ALLOC);
-	}
-	return p;
-}
-
-static inline void *
-fz_keep_imp16(fz_context *ctx, void *p, int16_t *refs)
-{
-	if (p)
-	{
-		(void)Memento_checkShortPointerOrNull(refs);
-		fz_lock(ctx, FZ_LOCK_ALLOC);
-		if (*refs > 0)
-		{
-			(void)Memento_takeRef(p);
-			++*refs;
-		}
-		fz_unlock(ctx, FZ_LOCK_ALLOC);
-	}
-	return p;
-}
-
-static inline int
-fz_drop_imp(fz_context *ctx, void *p, int *refs)
-{
-	if (p)
-	{
-		int drop;
-		(void)Memento_checkIntPointerOrNull(refs);
-		fz_lock(ctx, FZ_LOCK_ALLOC);
-		if (*refs > 0)
-		{
-			(void)Memento_dropIntRef(p);
-			drop = --*refs == 0;
-		}
-		else
-			drop = 0;
-		fz_unlock(ctx, FZ_LOCK_ALLOC);
-		return drop;
-	}
-	return 0;
-}
-
-static inline int
-fz_drop_imp8(fz_context *ctx, void *p, int8_t *refs)
-{
-	if (p)
-	{
-		int drop;
-		(void)Memento_checkBytePointerOrNull(refs);
-		fz_lock(ctx, FZ_LOCK_ALLOC);
-		if (*refs > 0)
-		{
-			(void)Memento_dropByteRef(p);
-			drop = --*refs == 0;
-		}
-		else
-			drop = 0;
-		fz_unlock(ctx, FZ_LOCK_ALLOC);
-		return drop;
-	}
-	return 0;
-}
-
-static inline int
-fz_drop_imp16(fz_context *ctx, void *p, int16_t *refs)
-{
-	if (p)
-	{
-		int drop;
-		(void)Memento_checkShortPointerOrNull(refs);
-		fz_lock(ctx, FZ_LOCK_ALLOC);
-		if (*refs > 0)
-		{
-			(void)Memento_dropShortRef(p);
-			drop = --*refs == 0;
-		}
-		else
-			drop = 0;
-		fz_unlock(ctx, FZ_LOCK_ALLOC);
-		return drop;
-	}
-	return 0;
-}
-
 #endif
diff --git a/source/fitz/bitmap.c b/source/fitz/bitmap.c
index fe16759c..88e37daf 100644
--- a/source/fitz/bitmap.c
+++ b/source/fitz/bitmap.c
@@ -1,4 +1,5 @@
 #include "mupdf/fitz.h"
+#include "fitz-imp.h"
 
 #include <string.h>
 
diff --git a/source/fitz/context.c b/source/fitz/context.c
index b342e6e1..f4ebdc98 100644
--- a/source/fitz/context.c
+++ b/source/fitz/context.c
@@ -181,7 +181,7 @@ new_context_phase1(const fz_alloc_context *alloc, const fz_locks_context *locks)
 	memset(ctx, 0, sizeof *ctx);
 	ctx->user = NULL;
 	ctx->alloc = alloc;
-	ctx->locks = locks;
+	ctx->locks = *locks;
 
 	ctx->glyph_cache = NULL;
 
@@ -265,7 +265,7 @@ fz_clone_context(fz_context *ctx)
 {
 	/* We cannot safely clone the context without having locking/
 	 * unlocking functions. */
-	if (ctx == NULL || ctx->locks == &fz_locks_default)
+	if (ctx == NULL || (ctx->locks.lock == fz_locks_default.lock && ctx->locks.unlock == fz_locks_default.unlock))
 		return NULL;
 	return fz_clone_context_internal(ctx);
 }
@@ -278,7 +278,7 @@ fz_clone_context_internal(fz_context *ctx)
 	if (ctx == NULL || ctx->alloc == NULL)
 		return NULL;
 
-	new_ctx = new_context_phase1(ctx->alloc, ctx->locks);
+	new_ctx = new_context_phase1(ctx->alloc, &ctx->locks);
 	if (!new_ctx)
 		return NULL;
 
diff --git a/source/fitz/document.c b/source/fitz/document.c
index 6f9d1316..2b494c62 100644
--- a/source/fitz/document.c
+++ b/source/fitz/document.c
@@ -1,4 +1,5 @@
 #include "mupdf/fitz.h"
+#include "fitz-imp.h"
 
 #include <string.h>
 
diff --git a/source/fitz/draw-glyph.c b/source/fitz/draw-glyph.c
index 9c0a1b70..75bfd616 100644
--- a/source/fitz/draw-glyph.c
+++ b/source/fitz/draw-glyph.c
@@ -1,6 +1,7 @@
 #include "mupdf/fitz.h"
 #include "draw-imp.h"
 #include "glyph-cache-imp.h"
+#include "fitz-imp.h"
 
 #include <string.h>
 #include <math.h>
diff --git a/source/fitz/fitz-imp.h b/source/fitz/fitz-imp.h
index 23908a8d..41c905e4 100644
--- a/source/fitz/fitz-imp.h
+++ b/source/fitz/fitz-imp.h
@@ -91,4 +91,152 @@ void fz_new_output_context(fz_context *ctx);
 void fz_drop_output_context(fz_context *ctx);
 fz_output_context *fz_keep_output_context(fz_context *ctx);
 
+#if defined(MEMENTO) || !defined(NDEBUG)
+#define FITZ_DEBUG_LOCKING
+#endif
+
+#ifdef FITZ_DEBUG_LOCKING
+
+void fz_assert_lock_held(fz_context *ctx, int lock);
+void fz_assert_lock_not_held(fz_context *ctx, int lock);
+void fz_lock_debug_lock(fz_context *ctx, int lock);
+void fz_lock_debug_unlock(fz_context *ctx, int lock);
+
+#else
+
+#define fz_assert_lock_held(A,B) do { } while (0)
+#define fz_assert_lock_not_held(A,B) do { } while (0)
+#define fz_lock_debug_lock(A,B) do { } while (0)
+#define fz_lock_debug_unlock(A,B) do { } while (0)
+
+#endif /* !FITZ_DEBUG_LOCKING */
+
+static inline void
+fz_lock(fz_context *ctx, int lock)
+{
+	fz_lock_debug_lock(ctx, lock);
+	ctx->locks.lock(ctx->locks.user, lock);
+}
+
+static inline void
+fz_unlock(fz_context *ctx, int lock)
+{
+	fz_lock_debug_unlock(ctx, lock);
+	ctx->locks.unlock(ctx->locks.user, lock);
+}
+
+static inline void *
+fz_keep_imp(fz_context *ctx, void *p, int *refs)
+{
+	if (p)
+	{
+		(void)Memento_checkIntPointerOrNull(refs);
+		fz_lock(ctx, FZ_LOCK_ALLOC);
+		if (*refs > 0)
+		{
+			(void)Memento_takeRef(p);
+			++*refs;
+		}
+		fz_unlock(ctx, FZ_LOCK_ALLOC);
+	}
+	return p;
+}
+
+static inline void *
+fz_keep_imp8(fz_context *ctx, void *p, int8_t *refs)
+{
+	if (p)
+	{
+		(void)Memento_checkBytePointerOrNull(refs);
+		fz_lock(ctx, FZ_LOCK_ALLOC);
+		if (*refs > 0)
+		{
+			(void)Memento_takeRef(p);
+			++*refs;
+		}
+		fz_unlock(ctx, FZ_LOCK_ALLOC);
+	}
+	return p;
+}
+
+static inline void *
+fz_keep_imp16(fz_context *ctx, void *p, int16_t *refs)
+{
+	if (p)
+	{
+		(void)Memento_checkShortPointerOrNull(refs);
+		fz_lock(ctx, FZ_LOCK_ALLOC);
+		if (*refs > 0)
+		{
+			(void)Memento_takeRef(p);
+			++*refs;
+		}
+		fz_unlock(ctx, FZ_LOCK_ALLOC);
+	}
+	return p;
+}
+
+static inline int
+fz_drop_imp(fz_context *ctx, void *p, int *refs)
+{
+	if (p)
+	{
+		int drop;
+		(void)Memento_checkIntPointerOrNull(refs);
+		fz_lock(ctx, FZ_LOCK_ALLOC);
+		if (*refs > 0)
+		{
+			(void)Memento_dropIntRef(p);
+			drop = --*refs == 0;
+		}
+		else
+			drop = 0;
+		fz_unlock(ctx, FZ_LOCK_ALLOC);
+		return drop;
+	}
+	return 0;
+}
+
+static inline int
+fz_drop_imp8(fz_context *ctx, void *p, int8_t *refs)
+{
+	if (p)
+	{
+		int drop;
+		(void)Memento_checkBytePointerOrNull(refs);
+		fz_lock(ctx, FZ_LOCK_ALLOC);
+		if (*refs > 0)
+		{
+			(void)Memento_dropByteRef(p);
+			drop = --*refs == 0;
+		}
+		else
+			drop = 0;
+		fz_unlock(ctx, FZ_LOCK_ALLOC);
+		return drop;
+	}
+	return 0;
+}
+
+static inline int
+fz_drop_imp16(fz_context *ctx, void *p, int16_t *refs)
+{
+	if (p)
+	{
+		int drop;
+		(void)Memento_checkShortPointerOrNull(refs);
+		fz_lock(ctx, FZ_LOCK_ALLOC);
+		if (*refs > 0)
+		{
+			(void)Memento_dropShortRef(p);
+			drop = --*refs == 0;
+		}
+		else
+			drop = 0;
+		fz_unlock(ctx, FZ_LOCK_ALLOC);
+		return drop;
+	}
+	return 0;
+}
+
 #endif
diff --git a/source/fitz/harfbuzz.c b/source/fitz/harfbuzz.c
index be70cdd6..03e2b62f 100644
--- a/source/fitz/harfbuzz.c
+++ b/source/fitz/harfbuzz.c
@@ -4,6 +4,7 @@
  */
 
 #include "mupdf/fitz.h"
+#include "fitz-imp.h"
 
 #include "hb.h"
 
diff --git a/source/fitz/hash.c b/source/fitz/hash.c
index af95ba9c..669f5d2c 100644
--- a/source/fitz/hash.c
+++ b/source/fitz/hash.c
@@ -1,4 +1,5 @@
 #include "mupdf/fitz.h"
+#include "fitz-imp.h"
 
 #include <string.h>
 #include <assert.h>
diff --git a/source/fitz/link.c b/source/fitz/link.c
index 9f8286be..b47fa9ee 100644
--- a/source/fitz/link.c
+++ b/source/fitz/link.c
@@ -1,4 +1,5 @@
 #include "mupdf/fitz.h"
+#include "fitz-imp.h"
 
 fz_link *
 fz_new_link(fz_context *ctx, const fz_rect *bbox, void *doc, const char *uri)
diff --git a/source/fitz/load-jpx.c b/source/fitz/load-jpx.c
index 3d7735d5..6730a645 100644
--- a/source/fitz/load-jpx.c
+++ b/source/fitz/load-jpx.c
@@ -1,4 +1,5 @@
 #include "mupdf/fitz.h"
+#include "fitz-imp.h"
 
 #include <assert.h>
 #include <string.h>
diff --git a/source/fitz/memory.c b/source/fitz/memory.c
index b935853d..41543112 100644
--- a/source/fitz/memory.c
+++ b/source/fitz/memory.c
@@ -1,4 +1,5 @@
 #include "mupdf/fitz.h"
+#include "fitz-imp.h"
 
 #include <string.h>
 #include <stdlib.h>
@@ -326,7 +327,7 @@ static int find_context(fz_context *ctx)
 			 * threads trying here too though so, so claim it
 			 * atomically. No one has locked on this context
 			 * before, so we are safe to take the ALLOC lock. */
-			ctx->locks->lock(ctx->locks->user, FZ_LOCK_ALLOC);
+			ctx->locks.lock(ctx->locks.user, FZ_LOCK_ALLOC);
 			/* If it's still free, then claim it as ours,
 			 * otherwise we'll keep hunting. */
 			if (fz_lock_debug_contexts[i] == NULL)
@@ -342,7 +343,7 @@ static int find_context(fz_context *ctx)
 				}
 #endif
 			}
-			ctx->locks->unlock(ctx->locks->user, FZ_LOCK_ALLOC);
+			ctx->locks.unlock(ctx->locks.user, FZ_LOCK_ALLOC);
 			if (gottit)
 				return i;
 		}
diff --git a/source/fitz/outline.c b/source/fitz/outline.c
index ba894ab0..395e7af5 100644
--- a/source/fitz/outline.c
+++ b/source/fitz/outline.c
@@ -1,4 +1,5 @@
 #include "mupdf/fitz.h"
+#include "fitz-imp.h"
 
 fz_outline *
 fz_new_outline(fz_context *ctx)
diff --git a/source/fitz/path.c b/source/fitz/path.c
index b56d7b59..92af16f7 100644
--- a/source/fitz/path.c
+++ b/source/fitz/path.c
@@ -1,4 +1,5 @@
 #include "mupdf/fitz.h"
+#include "fitz-imp.h"
 
 #include <string.h>
 #include <assert.h>
diff --git a/source/fitz/separation.c b/source/fitz/separation.c
index 82f6ead7..ee3c35c7 100644
--- a/source/fitz/separation.c
+++ b/source/fitz/separation.c
@@ -1,4 +1,5 @@
 #include "mupdf/fitz.h"
+#include "fitz-imp.h"
 
 #include <assert.h>
 #include <string.h>
diff --git a/source/fitz/store.c b/source/fitz/store.c
index e2303fd3..f94a8aa2 100644
--- a/source/fitz/store.c
+++ b/source/fitz/store.c
@@ -1,4 +1,5 @@
 #include "mupdf/fitz.h"
+#include "fitz-imp.h"
 
 #include <assert.h>
 #include <limits.h>
@@ -660,9 +661,7 @@ fz_empty_store(fz_context *ctx)
 	fz_lock(ctx, FZ_LOCK_ALLOC);
 	/* Run through all the items in the store */
 	while (store->head)
-	{
 		evict(ctx, store->head); /* Drops then retakes lock */
-	}
 	fz_unlock(ctx, FZ_LOCK_ALLOC);
 }
 
diff --git a/source/fitz/text.c b/source/fitz/text.c
index cf0b68be..8a01e550 100644
--- a/source/fitz/text.c
+++ b/source/fitz/text.c
@@ -1,4 +1,5 @@
 #include "mupdf/fitz.h"
+#include "fitz-imp.h"
 
 #include <string.h>
 
diff --git a/source/pdf/pdf-font.c b/source/pdf/pdf-font.c
index 96de61ef..f3a48f2d 100644
--- a/source/pdf/pdf-font.c
+++ b/source/pdf/pdf-font.c
@@ -2,6 +2,7 @@
 #include "mupdf/pdf.h"
 
 #include "../fitz/font-imp.h"
+#include "../fitz/fitz-imp.h"
 
 #include <assert.h>
 
diff --git a/source/pdf/pdf-graft.c b/source/pdf/pdf-graft.c
index 50454e40..50be9790 100644
--- a/source/pdf/pdf-graft.c
+++ b/source/pdf/pdf-graft.c
@@ -1,5 +1,6 @@
 #include "mupdf/fitz.h"
 #include "mupdf/pdf.h"
+#include "../fitz/fitz-imp.h"
 
 #include <assert.h>
 
diff --git a/source/pdf/pdf-object.c b/source/pdf/pdf-object.c
index 7788869e..816d7e3d 100644
--- a/source/pdf/pdf-object.c
+++ b/source/pdf/pdf-object.c
@@ -1,5 +1,6 @@
 #include "mupdf/fitz.h"
 #include "mupdf/pdf.h"
+#include "../fitz/fitz-imp.h"
 
 #include "pdf-name-table.h"
 
diff --git a/source/pdf/pdf-pkcs7.c b/source/pdf/pdf-pkcs7.c
index cab54742..403c3f7d 100644
--- a/source/pdf/pdf-pkcs7.c
+++ b/source/pdf/pdf-pkcs7.c
@@ -1,5 +1,6 @@
 #include "mupdf/fitz.h"
 #include "mupdf/pdf.h"
+#include "../fitz/fitz-imp.h"
 
 #include <string.h>
 
diff --git a/source/xps/xps-glyphs.c b/source/xps/xps-glyphs.c
index 5b26d780..31da84fd 100644
--- a/source/xps/xps-glyphs.c
+++ b/source/xps/xps-glyphs.c
@@ -1,5 +1,6 @@
 #include "mupdf/fitz.h"
 #include "xps-imp.h"
+#include "../fitz/fitz-imp.h"
 
 #include <ft2build.h>
 #include FT_FREETYPE_H