[gs-code-review] Memory leak from API calls (688181, ref: 688599) |

[Ray Johnston]

Stefan,

Since the 'gsapi_new_instance' would allocate a separate 'mem'
for each instance (putting the pointer to that 'mem' in its
minst->heap element), each gs_malloc_release will not touch
another thread's allocations.

Of course, as you are aware, gsapi_new_instance currently has
a lock-out preventing multiple instances:

     /* limited to 1 instance, till it works :) */
     if ( gsapi_instance_counter >= gsapi_instance_max )
	return e_Fatal;
     ++gsapi_instance_counter;

but I think the gs_malloc_release in gsapi_delete_instance
is compatible with multiple instance use.

There is a curious bit of code later in gsapi_new_instance:

     if (*pinstance == NULL)
	/* first instance in this process */
	mem = gs_malloc_init(NULL);
     else {
	/* nothing different for second thread initialization
	 * seperate memory, ids, only stdio is process shared.
	 */
	mem = gs_malloc_init(NULL);

IMHO, each new instance _should_ have its own allocator (as this
code will do since it always calls gs_malloc_init) and each
thread that starts an instance must maintain a separate
instance pointer so we should NEVER get called with *pinstance
other than NULL. Otherwise how will the application handle
communications with each thread? We probably should return
e_Fatal if *pinstance if ever not NULL and document that an
application that wants to re-use a pointer needs to set it to
NULL before calling gsapi_new_instance.

Regards,
Ray
____________________________________________________________________


Stefan Kemper wrote:
> The malloc release isn't what I had intended, this prevents multiple
> threads since it will blow another threads memory away.  Other than
> that it's fine.