move VipsRegion off VipsObject again

We were having various unfixable segvs with VipsRegion on top of
VipsObject. Move back to a simple struct to reestablish stability,
then try slowly moving back to GObject again.

TODO

@@ -35,6 +35,8 @@
 
   how dumb, common this up somehow
 
+  sink.c is the replacement for im_iterate()
+
 
 - decls for type_map etc. are now in object.h, move the code to object.c
 

libvips/include/vips/region.h

@@ -32,33 +32,16 @@
 
  */
 
-#ifndef IM_REGION_H
+#ifndef VIPS_REGION_H
-#define IM_REGION_H
+#define VIPS_REGION_H
 
 #ifdef __cplusplus
 extern "C" {
 #endif /*__cplusplus*/
 
-#define VIPS_TYPE_REGION (vips_region_get_type())
-#define VIPS_REGION( obj ) \
-	(G_TYPE_CHECK_INSTANCE_CAST( (obj), \
-	VIPS_TYPE_REGION, VipsRegion ))
-#define VIPS_REGION_CLASS( klass ) \
-	(G_TYPE_CHECK_CLASS_CAST( (klass), \
-	VIPS_TYPE_REGION, VipsRegionClass))
-#define VIPS_IS_REGION( obj ) \
-	(G_TYPE_CHECK_INSTANCE_TYPE( (obj), VIPS_TYPE_REGION ))
-#define VIPS_IS_REGION_CLASS( klass ) \
-	(G_TYPE_CHECK_CLASS_TYPE( (klass), VIPS_TYPE_REGION ))
-#define VIPS_REGION_GET_CLASS( obj ) \
-	(G_TYPE_INSTANCE_GET_CLASS( (obj), \
-	VIPS_TYPE_REGION, VipsRegionClass ))
-
 /* Sub-area of image.
  */
 typedef struct _VipsRegion {
-	VipsObject parent_object;
-
 	/*< public >*/
 	/* Users may read these two fields.
 	 */
@@ -92,14 +75,9 @@ typedef struct _VipsRegion {
 	gboolean invalid;	
 } VipsRegion;
 
-typedef struct _VipsRegionClass {
+void vips_region_free( VipsRegion *region );
-	VipsObjectClass parent_class;
-
-} VipsRegionClass;
-
-GType vips_region_get_type( void );
-
 VipsRegion *vips_region_new( VipsImage *im );
+void vips_region_print( VipsRegion *region, VipsBuf *buf );
 
 int vips_region_buffer( VipsRegion *reg, Rect *r );
 int vips_region_image( VipsRegion *reg, Rect *r );
@@ -164,4 +142,4 @@ int vips_region_prepare_many( VipsRegion **reg, Rect *r );
 }
 #endif /*__cplusplus*/
 
-#endif /*IM_REGION_H*/
+#endif /*VIPS_REGION_H*/

libvips/include/vips/vips7compat.h

@@ -180,7 +180,7 @@ extern "C" {
 #define im_prepare vips_region_prepare
 #define im_prepare_to vips_region_prepare_to
 #define im_region_create vips_region_new
-#define im_region_free g_object_unref
+#define im_region_free vips_region_free
 #define im_region_region vips_region_region
 #define im_region_buffer vips_region_buffer
 #define im_region_black vips_region_black

libvips/iofuncs/im_generate.c

@@ -149,7 +149,7 @@ im_stop_one( void *seq, void *a, void *b )
 {
 	VipsRegion *reg = (VipsRegion *) seq;
 
-	g_object_unref( reg );
+	vips_region_free( reg );
 
 	return( 0 );
 }
@@ -174,7 +174,7 @@ im_stop_many( void *seq, void *a, void *b )
 		int i;
 
 		for( i = 0; ar[i]; i++ )
-			g_object_unref( ar[i] );
+			vips_region_free( ar[i] );
 		im_free( (char *) ar );
 	}
 

libvips/iofuncs/image.c

@@ -1215,7 +1215,7 @@ vips_image_init( VipsImage *image )
 	image->magic = im_amiMSBfirst() ? VIPS_MAGIC_SPARC : VIPS_MAGIC_INTEL;
 
 	image->fd = -1;			/* since 0 is stdout */
-	image->sslock = g_mutex_new ();
+	image->sslock = g_mutex_new();
 
 	image->sizeof_header = IM_SIZEOF_HEADER;
 }

libvips/iofuncs/init.c

@@ -163,8 +163,6 @@ vips_init( const char *argv0 )
 	const char *prefix;
 	const char *libdir;
 	char name[256];
-	VipsImage *image;
-	VipsRegion *region;
 
 	/* Two stage done handling: 'done' means we've completed, 'started'
 	 * means we're currently initialising. Use this to prevent recursive
@@ -219,16 +217,6 @@ vips_init( const char *argv0 )
 	im__format_init();
 	vips__interpolate_init();
 
-	/* We make regions in parallel, so we have to be careful that any
-	 * associated types are fully built before we start. We can't init the
-	 * clases in two separate threads.
-	 *
-	 * We can't unref these two, since the last unref of  the last region
-	 * will finalize the class and trigger re-init.
-	 */
-	image = vips_image_new( "p" );
-	region = vips_region_new( image );
-
 	/* Load up any plugins in the vips libdir. We don't error on failure,
 	 * it's too annoying to have VIPS refuse to start because of a broken
 	 * plugin.

libvips/iofuncs/object.c

@@ -70,6 +70,7 @@ enum {
 /* Table of all objects, handy for debugging.
  */
 static GHashTable *vips__object_all = NULL;
+static GMutex *vips__object_all_lock = NULL;
 
 static guint vips_object_signals[SIG_LAST] = { 0 };
 
@@ -454,7 +455,9 @@ vips_object_finalize( GObject *gobject )
 
 	vips_object_close( object );
 
+	g_mutex_lock( vips__object_all_lock );
 	g_hash_table_remove( vips__object_all, object );
+	g_mutex_unlock( vips__object_all_lock );
 
 	G_OBJECT_CLASS( vips_object_parent_class )->finalize( gobject );
 
@@ -836,9 +839,11 @@ vips_object_class_init( VipsObjectClass *class )
 
 	GParamSpec *pspec;
 
-	if( !vips__object_all ) 
+	if( !vips__object_all ) {
 		vips__object_all = g_hash_table_new( 
 			g_direct_hash, g_direct_equal );
+		vips__object_all_lock = g_mutex_new();
+	}
 
 	gobject_class->dispose = vips_object_dispose;
 	gobject_class->finalize = vips_object_finalize;
@@ -920,7 +925,9 @@ vips_object_init( VipsObject *object )
 	vips_object_print( object );
 #endif /*DEBUG*/
 
+	g_mutex_lock( vips__object_all_lock );
 	g_hash_table_insert( vips__object_all, object, object );
+	g_mutex_unlock( vips__object_all_lock );
 }
 
 /* Add a vipsargument ... automate some stuff with this.
@@ -1227,9 +1234,11 @@ vips_object_map( VSListMap2Fn fn, void *a, void *b )
 	args.a = a;
 	args.b = b;
 	args.result = NULL;
+	g_mutex_lock( vips__object_all_lock );
 	if( vips__object_all )
 		g_hash_table_foreach( vips__object_all, 
 			(GHFunc) vips_object_map_sub, &args );
+	g_mutex_unlock( vips__object_all_lock );
 
 	return( args.result );
 }

libvips/iofuncs/package.c

@@ -979,7 +979,7 @@ build_args( im_function *fn, im_object *vargv, int argc, char **argv )
 static void
 region_local_image_cb( VipsImage *main, VipsRegion *reg )
 {
-	g_object_unref( reg );
+	vips_region_free( reg );
 }
 
 /* Make a region on sub, closed by callback on main.

libvips/iofuncs/region.c

@@ -74,6 +74,7 @@
 #define DEBUG_ENVIRONMENT 1
 #define DEBUG_CREATE
 #define DEBUG
+#define DEBUG_VIPS
  */
 
 #ifdef HAVE_CONFIG_H
@@ -178,25 +179,12 @@
  * Returns: The address of the top-left pixel in the region.
  */
 
-/* Properties.
+#ifdef DEBUG
+/* Track all regions here for debugging.
  */
-enum {
+static GSList *vips__region_all = NULL;
-	PROP_IMAGE = 1,
+#endif /*DEBUG*/
-	PROP_LAST
-}; 
 
-G_DEFINE_TYPE( VipsRegion, vips_region, VIPS_TYPE_OBJECT );
-
-static void
-vips_region_finalize( GObject *gobject )
-{
-#ifdef VIPS_DEBUG
-	VIPS_DEBUG_MSG( "vips_region_finalize: " );
-	vips_object_print( VIPS_OBJECT( gobject ) );
-#endif /*VIPS_DEBUG*/
-
-	G_OBJECT_CLASS( vips_region_parent_class )->finalize( gobject );
-}
 
 /* Call a start function if no sequence is running on this VipsRegion.
  */
@@ -263,18 +251,14 @@ vips_region_reset( VipsRegion *region )
 	region->invalid = FALSE;
 }
 
-static void
+void
-vips_region_dispose( GObject *gobject )
+vips_region_free( VipsRegion *region )
 {
-	VipsRegion *region = VIPS_REGION( gobject );
 	VipsImage *image = region->im;
 
-#ifdef VIPS_DEBUG
+	g_assert( image );
-	VIPS_DEBUG_MSG( "vips_region_dispose: " );
-	vips_object_print( VIPS_OBJECT( gobject ) );
-#endif /*VIPS_DEBUG*/
 
-	vips_object_preclose( VIPS_OBJECT( gobject ) );
+	VIPS_DEBUG_MSG( "vips_region_free: %p\n", region );
 
         /* Stop this sequence.
          */
@@ -291,14 +275,22 @@ vips_region_dispose( GObject *gobject )
 	g_mutex_unlock( image->sslock );
 	region->im = NULL;
 
-	G_OBJECT_CLASS( vips_region_parent_class )->dispose( gobject );
+	g_object_unref( image );
+
+	im_free( region );
+
+#ifdef DEBUG
+	g_mutex_lock( vips__global_lock );
+	g_assert( g_slist_find( vips__region_all, region ) );
+	vips__region_all = g_slist_remove( vips__region_all, region );
+	printf( "%d regions in vips\n", g_slist_length( vips__region_all ) );
+	g_mutex_unlock( vips__global_lock );
+#endif /*DEBUG*/
 }
 
-static void
+void
-vips_region_print( VipsObject *object, VipsBuf *buf )
+vips_region_print( VipsRegion *region, VipsBuf *buf )
 {
-	VipsRegion *region = VIPS_REGION( object );
-
 	vips_buf_appendf( buf, "VipsRegion: %p, ", region );
 	vips_buf_appendf( buf, "im = %p, ", region->im );
 	vips_buf_appendf( buf, "valid.left = %d, ", region->valid.left );
@@ -313,8 +305,6 @@ vips_region_print( VipsObject *object, VipsBuf *buf )
 	vips_buf_appendf( buf, "window = %p, ", region->window );
 	vips_buf_appendf( buf, "buffer = %p\n", region->buffer );
 	vips_buf_appendf( buf, "invalid = %d\n", region->invalid );
-
-	VIPS_OBJECT_CLASS( vips_region_parent_class )->print( object, buf );
 }
 
 /* If a region is being created in one thread (eg. the main thread) and then
@@ -374,16 +364,38 @@ vips__region_no_ownership( VipsRegion *region )
 	g_mutex_unlock( region->im->sslock );
 }
 
-static int
+/**
-vips_region_build( VipsObject *object )
+ * vips_region_new:
+ * @image: image to create this region on
+ *
+ * Create a region. #VipsRegion s start out empty, you need to call 
+ * vips_region_prepare() to fill them with pixels.
+ *
+ * See also: vips_region_free(), vips_region_prepare().
+ */
+VipsRegion *
+vips_region_new( VipsImage *image )
 {
-	VipsRegion *region = VIPS_REGION( object );
+	VipsRegion *region;
-	VipsImage *image = region->im;
 
-	VIPS_DEBUG_MSG( "vips_region_build: %p\n", region );
+	if( !(region = VIPS_NEW( NULL, VipsRegion )) )
+		return( NULL );
+	VIPS_DEBUG_MSG( "vips_region_new: %p\n", region );
 
-	if( VIPS_OBJECT_CLASS( vips_region_parent_class )->build( object ) )
+	region->im = image;
-		return( -1 );
+	g_object_ref( image );
+	region->type = VIPS_REGION_NONE;
+	region->valid.left = 0;
+	region->valid.top = 0;
+	region->valid.width = 0;
+	region->valid.height = 0;
+	region->data = NULL;
+	region->bpl = 0;
+	region->seq = NULL;
+	region->thread = NULL;
+	region->window = NULL;
+	region->buffer = NULL;
+	region->invalid = FALSE;
 
 	vips__region_take_ownership( region );
 
@@ -393,70 +405,14 @@ vips_region_build( VipsObject *object )
 	image->regions = g_slist_prepend( image->regions, region );
 	g_mutex_unlock( image->sslock );
 
-	return( 0 );
+#ifdef DEBUG
-}
+	g_mutex_lock( vips__global_lock );
+	vips__region_all = g_slist_prepend( vips__region_all, region );
+	printf( "%d regions in vips\n", g_slist_length( vips__region_all ) );
+	g_mutex_unlock( vips__global_lock );
+#endif /*DEBUG*/
 
-static void
+	return( region );
-vips_region_class_init( VipsRegionClass *class )
-{
-	GObjectClass *gobject_class = G_OBJECT_CLASS( class );
-	VipsObjectClass *vobject_class = VIPS_OBJECT_CLASS( class );
-	GParamSpec *pspec;
-
-	gobject_class->finalize = vips_region_finalize;
-	gobject_class->dispose = vips_region_dispose;
-	gobject_class->set_property = vips_object_set_property;
-	gobject_class->get_property = vips_object_get_property;
-
-	vobject_class->print = vips_region_print;
-	vobject_class->build = vips_region_build;
-
-	/* Create properties.
-	 */
-	pspec = g_param_spec_object( "image", "Image",
-		_( "The image this region is defined upon" ),
-		VIPS_TYPE_IMAGE,
-		G_PARAM_READWRITE );
-	g_object_class_install_property( gobject_class, PROP_IMAGE, pspec );
-	vips_object_class_install_argument( vobject_class, pspec,
-		VIPS_ARGUMENT_REQUIRED_INPUT, 
-		G_STRUCT_OFFSET( VipsRegion, im ) );
-}
-
-static void
-vips_region_init( VipsRegion *region )
-{
-	/* Init to 0 is fine for most header fields. Others have default set
-	 * by property system.
-	 */
-
-	region->type = VIPS_REGION_NONE;
-}
-
-/**
- * vips_region_new:
- * @image: image to create this region on
- *
- * Create a region. #VipsRegion s start out empty, you need to call 
- * vips_region_prepare() to fill them with pixels.
- *
- * See also: vips_region_prepare().
- */
-VipsRegion *
-vips_region_new( VipsImage *image )
-{
-	VipsRegion *region;
-
-	region = VIPS_REGION( g_object_new( VIPS_TYPE_REGION, NULL ) );
-	g_object_set( region,
-		"image", image,
-		NULL );
-	if( vips_object_build( VIPS_OBJECT( region ) ) ) {
-		VIPS_UNREF( region );
-		return( NULL );
-	}
-
-	return( region ); 
 }
 
 /* Region should be a pixel buffer. On return, check

libvips/iofuncs/sink.c

@@ -131,7 +131,7 @@ sink_thread_state_dispose( GObject *gobject )
 	Sink *sink = (Sink *) ((VipsThreadState *) state)->a;
 
 	sink_call_stop( sink, state );
-	VIPS_FREEF( g_object_unref, state->reg );
+	VIPS_FREEF( vips_region_free, state->reg );
 
 	G_OBJECT_CLASS( sink_thread_state_parent_class )->dispose( gobject );
 }

libvips/iofuncs/sinkdisc.c

@@ -168,7 +168,7 @@ wbuffer_free( WriteBuffer *wbuffer )
 		wbuffer->thread = NULL;
         }
 
-	VIPS_FREEF( g_object_unref, wbuffer->region );
+	VIPS_FREEF( vips_region_free, wbuffer->region );
 	im_semaphore_destroy( &wbuffer->go );
 	im_semaphore_destroy( &wbuffer->nwrite );
 	im_semaphore_destroy( &wbuffer->done );

libvips/iofuncs/sinkmemory.c

@@ -75,7 +75,7 @@ typedef struct _Sink {
 static void
 sink_free( Sink *sink )
 {
-	VIPS_FREEF( g_object_unref, sink->all );
+	VIPS_FREEF( vips_region_free, sink->all );
 }
 
 static int

libvips/iofuncs/sinkscreen.c

@@ -208,7 +208,7 @@ tile_free( Tile *tile )
 {
 	VIPS_DEBUG_MSG_AMBER( "tile_free\n" );
 
-	VIPS_FREEF( g_object_unref, tile->region );
+	VIPS_FREEF( vips_region_free, tile->region );
 	im_free( tile );
 
 	return( NULL );
@@ -962,7 +962,7 @@ image_stop( void *seq, void *a, void *b )
 {
 	VipsRegion *reg = (VipsRegion *) seq;
 
-	g_object_unref( reg );
+	vips_region_free( reg );
 
 	return( 0 );
 }

libvips/iofuncs/threadpool.c

@@ -255,7 +255,7 @@ vips_thread_state_dispose( GObject *gobject )
 
 	VIPS_DEBUG_MSG( "vips_thread_state_dispose:\n" );
 
-	VIPS_FREEF( g_object_unref, state->reg );
+	VIPS_FREEF( vips_region_free, state->reg );
 
 	G_OBJECT_CLASS( vips_thread_state_parent_class )->dispose( gobject );
 }