From: Tony Cook <tony@develop-help.com>
Date: Thu, 12 Jul 2012 13:00:22 +0000 (+1000)
Subject: implement reference counting for context objects
X-Git-Tag: v0.92_01~48
X-Git-Url: http://git.imager.perl.org/imager.git/commitdiff_plain/28da39eee1bc9863c6389931ea953bd049760b94

implement reference counting for context objects

This works, even with threading.
---

diff --git a/Imager.xs b/Imager.xs
index 87fbcc6a..8d680e91 100644
--- a/Imager.xs
+++ b/Imager.xs
@@ -29,13 +29,69 @@ extern "C" {
 
 #include "imperl.h"
 
-static im_context_t work_context;
+/*
+
+Context object management
+
+*/
+
+typedef im_context_t Imager__Context;
+
+#define im_context_DESTROY(ctx) im_context_refdec((ctx), "DESTROY")
+
+#ifdef PERL_IMPLICIT_CONTEXT
+
+#define MY_CXT_KEY "Imager::_context" XS_VERSION
+
+typedef struct {
+  im_context_t ctx;
+} my_cxt_t;
+
+START_MY_CXT
+
+im_context_t fallback_context;
+
+static void
+start_context(pTHX) {
+  dMY_CXT;
+  MY_CXT.ctx = im_context_new();
+  sv_setref_pv(get_sv("Imager::_context", GV_ADD), "Imager::Context", MY_CXT.ctx);
+
+  /* Ideally we'd free this reference, but the error message memory
+     was never released on exit, so the associated memory here is reasonable
+     to keep.
+     With logging enabled we always need at least one context, since
+     objects may be released fairly late and attempt to get the log file.
+  */
+  im_context_refinc(MY_CXT.ctx, "start_context");
+  fallback_context = MY_CXT.ctx;
+}
+
+static im_context_t
+perl_get_context(void) {
+  dTHX;
+  dMY_CXT;
+  
+  return MY_CXT.ctx ? MY_CXT.ctx : fallback_context;
+}
+
+#else
+
+static im_context_t perl_context;
+
+static void
+start_context(pTHX) {
+  perl_context = im_context_new();
+  im_context_refinc(perl_context, "start_context");
+}
 
 static im_context_t
 perl_get_context(void) {
-  return work_context;
+  return perl_context;
 }
 
+#endif
+
 /* used to represent channel lists parameters */
 typedef struct i_channel_list_tag {
   int *channels;
@@ -734,7 +790,6 @@ validate_i_ppal(i_img *im, i_palidx const *indexes, int count) {
   }
 }
 
-
 /* I don't think ICLF_* names belong at the C interface
    this makes the XS code think we have them, to let us avoid 
    putting function bodies in the XS code
@@ -3996,8 +4051,28 @@ i_int_hlines_CLONE_SKIP(cls)
 
 #endif
 
+MODULE = Imager  PACKAGE = Imager::Context PREFIX=im_context_
+
+void
+im_context_DESTROY(ctx)
+   Imager::Context ctx
+
+#ifdef PERL_IMPLICIT_CONTEXT
+
+void
+im_context_CLONE(...)
+    CODE:
+      MY_CXT_CLONE;
+      /* the following sv_setref_pv() will free this inc */
+      im_context_refinc(MY_CXT.ctx, "CLONE");
+      MY_CXT.ctx = im_context_clone(MY_CXT.ctx, "CLONE");
+      sv_setref_pv(get_sv("Imager::_context", GV_ADD), "Imager::Context", MY_CXT.ctx);
+
+#endif
+
 BOOT:
         PERL_SET_GLOBAL_CALLBACKS;
 	PERL_PL_SET_GLOBAL_CALLBACKS;
-	work_context = im_context_new();
+        MY_CXT_INIT;
+	start_context(aTHX);
 	im_get_context = perl_get_context;
diff --git a/Makefile.PL b/Makefile.PL
index 926e1761..a3ab1d45 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -48,6 +48,7 @@ my @incpaths; # places to look for headers
 my @libpaths; # places to look for libraries
 my $coverage; # build for coverage testing
 my $assert; # build with assertions
+my $trace_context; # trace context management to stderr
 GetOptions("help" => \$help,
            "enable=s" => \@enable,
            "disable=s" => \@disable,
@@ -56,7 +57,8 @@ GetOptions("help" => \$help,
            "verbose|v" => \$VERBOSE,
            "nolog" => \$NOLOG,
 	   'coverage' => \$coverage,
-	   "assert|a" => \$assert);
+	   "assert|a" => \$assert,
+	   "tracecontext" => \$trace_context);
 
 setenv();
 
@@ -172,6 +174,10 @@ if ($] < 5.008) {
     unshift @typemaps, "typemap.oldperl";
 }
 
+if ($trace_context) {
+  $CFLAGS .= " -DIMAGER_TRACE_CONTEXT";
+}
+
 my %opts=
   (
    'NAME'         => 'Imager',
diff --git a/context.c b/context.c
index 9967c66b..4852bf81 100644
--- a/context.c
+++ b/context.c
@@ -1,4 +1,5 @@
 #include "imageri.h"
+#include <stdio.h>
 
 /*
 =item im_context_new()
@@ -30,11 +31,35 @@ im_context_new(void) {
   ctx->max_height = 0;
   ctx->max_bytes = DEF_BYTES_LIMIT;
 
+  ctx->refcount = 1;
+
+#ifdef IMAGER_TRACE_CONTEXT
+  fprintf(stderr, "im_context: created %p\n", ctx);
+#endif
+
   return ctx;
 }
 
 /*
-=item im_context_delete(ctx)
+=item im_context_refinc(ctx, where)
+
+Add a new reference to the context.
+
+=cut
+*/
+
+void
+im_context_refinc(im_context_t ctx, const char *where) {
+  ++ctx->refcount;
+
+#ifdef IMAGER_TRACE_CONTEXT
+  fprintf(stderr, "im_context:%s: refinc %p (count now %lu)\n", where,
+	  ctx, (unsigned long)ctx->refcount);
+#endif
+}
+
+/*
+=item im_context_refdec(ctx)
 
 Release memory used by an Imager context object.
 
@@ -42,9 +67,21 @@ Release memory used by an Imager context object.
 */
 
 void
-im_context_delete(im_context_t ctx) {
+im_context_refdec(im_context_t ctx, const char *where) {
   int i;
 
+  im_assert(ctx->refcount > 0);
+
+  --ctx->refcount;
+
+#ifdef IMAGER_TRACE_CONTEXT
+  fprintf(stderr, "im_context:%s: delete %p (count now %lu)\n", where,
+	  ctx, (unsigned long)ctx->refcount);
+#endif
+
+  if (ctx->refcount != 0)
+    return;
+
   for (i = 0; i < IM_ERROR_COUNT; ++i) {
     if (ctx->error_stack[i].msg)
       myfree(ctx->error_stack[i].msg);
@@ -66,7 +103,7 @@ Clone an Imager context object, returning the result.
 */
 
 im_context_t
-im_context_clone(im_context_t ctx) {
+im_context_clone(im_context_t ctx, const char *where) {
   im_context_t nctx = malloc(sizeof(im_context_struct));
   int i;
 
@@ -105,6 +142,11 @@ im_context_clone(im_context_t ctx) {
   nctx->max_width = ctx->max_width;
   nctx->max_height = ctx->max_height;
   nctx->max_bytes = ctx->max_bytes;
+  nctx->refcount = 1;
 
-  return ctx;
+#ifdef IMAGER_TRACE_CONTEXT
+  fprintf(stderr, "im_context:%s: cloned %p to %p\n", where, ctx, nctx);
+#endif
+
+  return nctx;
 }
diff --git a/image.c b/image.c
index 5c602d75..600b529a 100644
--- a/image.c
+++ b/image.c
@@ -94,6 +94,7 @@ void
 im_img_init(pIMCTX, i_img *img) {
   img->im_data = NULL;
   img->context = aIMCTX;
+  im_context_refinc(aIMCTX, "img_init");
 }
 
 /* 
@@ -274,9 +275,11 @@ Destroy an image object
 
 void
 i_img_destroy(i_img *im) {
+  dIMCTXim(im);
   mm_log((1,"i_img_destroy(im %p)\n",im));
   i_img_exorcise(im);
   if (im) { myfree(im); }
+  im_context_refdec(aIMCTX, "img_destroy");
 }
 
 /* 
diff --git a/imager.h b/imager.h
index cb38a145..0d6523f6 100644
--- a/imager.h
+++ b/imager.h
@@ -383,8 +383,9 @@ i_gsampf_bg(i_img *im, i_img_dim l, i_img_dim r, i_img_dim y, i_fsample_t *sampl
 
 /* context object management */
 extern im_context_t im_context_new(void);
-extern void im_context_delete(im_context_t ctx);
-extern im_context_t im_context_clone(im_context_t ctx);
+extern void im_context_refinc(im_context_t ctx, const char *where);
+extern void im_context_refdec(im_context_t ctx, const char *where);
+extern im_context_t im_context_clone(im_context_t ctx, const char *where);
 
 extern im_context_t (*im_get_context)(void);
 
diff --git a/imageri.h b/imageri.h
index c563c458..59d333a7 100644
--- a/imageri.h
+++ b/imageri.h
@@ -6,6 +6,7 @@
 #define IMAGEI_H_
 
 #include "imager.h"
+#include <stddef.h>
 
 /* wrapper functions that implement the floating point sample version of a 
    function in terms of the 8-bit sample version
@@ -126,6 +127,8 @@ typedef struct im_context_tag {
   /* file size limits */
   i_img_dim max_width, max_height;
   size_t max_bytes;
+
+  ptrdiff_t refcount;
 } im_context_struct;
 
 #define DEF_BYTES_LIMIT 0x40000000
diff --git a/typemap.local b/typemap.local
index 6ad73ff4..c738af77 100644
--- a/typemap.local
+++ b/typemap.local
@@ -10,6 +10,8 @@ off_t			T_OFF_T
 
 Imager::Internal::Hlines T_PTROBJ
 
+Imager::Context		 T_PTROBJ
+
 #############################################################################
 INPUT