Also can you verify that the gsmchunk.c is NOT threadsafe? I'm experimenting with a asynchronous interpreter/rendering rip, and am seeing e.g. chunk_mem_node_add being called from multiple threads at the same time.

(In reply to comment #1)
> Also can you verify that the gsmchunk.c is NOT threadsafe? I'm experimenting
> with a asynchronous interpreter/rendering rip, and am seeing e.g.
> chunk_mem_node_add being called from multiple threads at the same time.

And if NOT threadsafe, can it be made threadsafe?

I'll take this one. Thanks for spotting this.

As far as thread safety, the chunk allocator is not thread safe, but a thread
safe allocator can be constructed by wrapping it in a 'locked' allocator.

For example:

gs_memory_t *
chunk_mem_lock(gs_memory_t *chunk_mem);
{
    locked_chunk_mem = (gs_memory_t *) gs_alloc_bytes(chunk_mem,
                            sizeof(gs_memory_locked_t),
		            "chunk_mem_lock(locked_chunk_mem)");
   if (locked_chunk_mem == NULL ||
       (code = gs_memory_locked_init((gs_memory_locked_t *)locked_chunk_mem,
                                      chunk_mem)) < 0)
	 return NULL;
    return locked_chunk_mem;
}

Note that locking memory on _every_ request can have performance penalties,
which is why it is generally better to give threads their own chunk allocator,
with the underlying allocator being thread safe (as gxclthrd.c does).

Created attachment 6844 [details]
gsmchunk.c

layout of gs_memory_chunk_t modified to match the layout for the gs_malloc_memory_t. Thus used and max_used are the same and can be used for memory-usage logging (enabled with define MY_CODE).

(In reply to comment #3)

> 
> Note that locking memory on _every_ request can have performance penalties,
> which is why it is generally better to give threads their own chunk allocator,
> with the underlying allocator being thread safe (as gxclthrd.c does).

I have a bitdevice setup with async interpretation and rendering (i.e. a
writerdevice (for the interpreter) and a readerdevice for the rendering of the
bitmap. I use gp_create_thread() to start the reader_thread, and then block
until the reader_thread is done rendering.  (I.e. the interpreter does not
continue until rendering is done, real asynchronous interpreting/rendering is
not yet enabled).
The reader_thread then sets up 2 threads for renderingbands simultateously.
I have a problem that both a renderthread (band 0) and the reader_thread
(during setup for the renderthread for band 1) are calling chunk_obj_alloc(with
the same chunk_allocator (see attached callstack_2renderthread.txt (with
accompanying, slightly modified, gsmchunk.c)

Created attachment 6845 [details]
callstack_2renderthread.txt

callstack of 2 threads.

Created attachment 6846 [details]
callstack_2renderthread.txt

extra info added

both threads do a chunk_obj_alloc(renderbandthread for 76170bytes, renderbandsetup for 12MB). they both have the cmem->target set to the same allocator.
so chunk_obj_alloc calls chunk_mem_node_add(cmem, ...)
and chunk_mem_node_add calls chunk_alloc_bytes_immovable(cmem->target, ...)
for both calls of chunk_mem_node_add the cmem is different, but the cmem->target is the same. Shouldn't this be semaphored/locked?

(In reply to comment #8)
> both threads do a chunk_obj_alloc(renderbandthread for 76170bytes,
> renderbandsetup for 12MB). they both have the cmem->target set to the same
> allocator.
> so chunk_obj_alloc calls chunk_mem_node_add(cmem, ...)
> and chunk_mem_node_add calls gs_alloc_bytes_immovable(cmem->target, ...)
> for both calls of chunk_mem_node_add the cmem is different, but the
> cmem->target is the same. Shouldn't this be semaphored/locked?

typo: chunk_alloc_bytes_immovable is gs_alloc_bytes_immovable

Thanks, Norbert. I committed the fix for the increment of 'used'.

I did _not_ make the change to "align" the elements of the two different
memory structures since casting one to the other is too likely to cause
problems if one or the other of the structures is changed in the future,
and we would not like to constrain the definition of these.

I believe I addressed the issues with the thread safety of the chunk allocator,
specifically, the allocator is NOT thread safe by itself, but can be made
so by using a 'locking wrapper' allocator. Also the 'base' allocator of the
chunk allocator (or any allocator) MUST BE thread safe to be used as the
base allocator by more than one thread (as the multi-threaded rendering does).

ONLY the "heap allocator" (gsmalloc.c) is inherently thread safe since it
has a 'monitor' (mutex) for all operations that affect its internal structs.

Any further questions can be addressed by email to support.