Is malloc/free thread-safe?

Discuss the development of new homebrew software, tools and libraries.

Moderators: cheriff, TyRaNiD

Post Reply
Sharkus
Posts: 27
Joined: Sun Jun 19, 2005 6:49 am

Is malloc/free thread-safe?

Post by Sharkus »

I'm having a problem that looks to be caused by a multi-threaded SDL application calling into SDL functions in a re-entrant fasion (which in turn causes calls to malloc and free re-entrantly). (Not sure if "re-entrantly" is a word?)

I believe that SDL assumes that malloc/free (and prehaps other libc/os supplied functions) are thread-safe.

Looking through the newlib code it looks like __malloc_lock and __malloc_unlock are basically stubs at this point (at least in newlib/libc/sys/psp/libcglue.c). Am I looking at the right place?

-Sharkus
TyRaNiD
Posts: 907
Joined: Sun Jan 18, 2004 12:23 am

Post by TyRaNiD »

Oh yes, forget to finish that up :) Will put it on my todo list.

malloc/free will not be thread safe.
Sharkus
Posts: 27
Joined: Sun Jun 19, 2005 6:49 am

Post by Sharkus »

Hi TyRaNiD,

Are you saying it will NEVER be thread safe (even after you finish your to-do list)?

If that is the case then thread support should be removed from the PSP SDL implementation (similiar to OS 9).

http://www.libsdl.org/faq.php?action=li ... egory=6#59
TyRaNiD
Posts: 907
Joined: Sun Jan 18, 2004 12:23 am

Post by TyRaNiD »

I just mean the current implementation wont be thread safe as they stand, I can make them thread safe I think :)
Sharkus
Posts: 27
Joined: Sun Jun 19, 2005 6:49 am

Post by Sharkus »

Would something along these lines work?

Code: Select all

static SceUID  heap_sem = -1;
static int lockid = -1;
static int locks;

void __malloc_lock ( struct _reent *_r )
{
	int id;

	/* assume our first call is not reentrant or we can init this elsewhere */
	if( heap_sem == -1 ) {
		heap_sem = sceKernelCreateSema("heap sem", 0, 0, 255, NULL);
	}

	id = sceKernelGetThreadId();
	if( id == lockid ) {
		/* The cur thread already owns the semaphore so just increment locks */
		locks++;
	}
	else {
		/* Wait for the semaphore to be released */
		sceKernelWaitSema(heap_sem, 1, NULL);
		lockid = id;
	}
}


void __malloc_unlock ( struct _reent *_r )
{
	locks--;
	if( locks == 0 ) {
		lockid = -1;
		sceKernelSignalSema(heap_sem, 1);
	}
}
TyRaNiD
Posts: 907
Joined: Sun Jan 18, 2004 12:23 am

Post by TyRaNiD »

That looks like it would probably work, there is still a possible race condition of setting up the mutex.

Apparently how Sony do it (and MrBrown originally implemented it) is to disable interrupts at lock time and then reenable them afterwards, this prevents any threads pre-empting your allocations. MrBrown however found out that the respective functions don't get linked into kernel apps which made it useless for SDL anyway :)

However pspsdk has not implemented its own functions to do it with pspSdkDisableInterrupts and pspSdkEnableInterrupts. Those might do the trick for you.
Sharkus
Posts: 27
Joined: Sun Jun 19, 2005 6:49 am

Post by Sharkus »

oops. bug in my code below caused things to hang instantly. not very fun dugging that. A couple general questions regarding fun with newlibc:

1. Do you really have to do clean recompiles of newlib, pspsdk, SDL and the app for each change to newlib? If I didn't do this I typically the fix-imports would fail.

2. What type of debugging output is available in newlib? I tried printf, fprintf(stderr), and Kprintf. Nothing appeared on the PSPLINK console (I disable io redirect)

Fixed code is below (ready for integration):

Code: Select all

static SceUID  heap_sem = -1;
static int lockid = -1;
static int locks;

void __malloc_lock ( struct _reent *_r )
{
   int id;

   /* assume our first call to malloc will not be re-entrant */
   if( heap_sem == -1 ) {
      heap_sem = sceKernelCreateSema("heap sem", 0, 1, 255, NULL);
   }

   id = sceKernelGetThreadId();
   if( id != lockid ) {
      /* Wait for the semaphore to be released */
      sceKernelWaitSema(heap_sem, 1, NULL);
      lockid = id;
   }
   locks++;
}

void __malloc_unlock ( struct _reent *_r )
{
   locks--;
   if( locks == 0 ) {
      lockid = -1;
      sceKernelSignalSema(heap_sem, 1);
   }
}
Sharkus
Posts: 27
Joined: Sun Jun 19, 2005 6:49 am

Post by Sharkus »

What's funny is that after all of this re-entrant malloc/free was not my main problem... It was a stack overflow. (I guess it is only funny because it is 3:00 am and I have to be to work in a few hours.)

Any bright ideas on how to make the SDL thread stack size programmable at run time (or is it better just to hack the code on an application by application basis)?
jimparis
Posts: 1145
Joined: Fri Jun 10, 2005 4:21 am
Location: Boston

Post by jimparis »

Use an environment variable if you want to override the default at runtime, although I'd also be in favor just making the default 48k or 64k or something if you think that's a generally useful change.

For that malloc locking -- you should probably create the semaphore in __psp_libc_init, to avoid that initial race condition.
Sharkus
Posts: 27
Joined: Sun Jun 19, 2005 6:49 am

Post by Sharkus »

Hopefully going to have some time to start my PSP devel effort again. Does someone want to check in a couple updates or is it better for me to get SVN access?
bitachou
Posts: 1
Joined: Fri Jun 05, 2009 2:03 am

Post by bitachou »

Hi everybody,

(I'm new in this forum and I don't know where is the presentaion topic if there is one).

I don't know if I had to up this topic or create another one.

I've problems when I'm using thread.
When two(or more) threads are allocating memory "at the same time" I've always have hardware exceptions.
It's not the same each time, it could be adresse storz or load/inst fetch or bus error.

But with the psplib debug and pspadress2line the error seems to be always on mallocr.c (but not each time at the same line).

So I take a look at the newlib source in pspsdk source and malloc , free and others seems to be thread safe (and reentrant) .

but with more search I found in newlib-1.16.0\newlib\libc\include\sys\lock.h that
#define __lock_acquire_recursive(lock) (0)

so __lock_acquire_recursive is not implemented so malloc is not thread safe at all

am I right ?

Did someone have already fix this problem ?

And last question :
Am I the only one who have this problem since 2006(the date o the last post on the topic) ?

Thank you

(PS: sorry for the ugly english but I's not my birth language )
J.F.
Posts: 2906
Joined: Sun Feb 22, 2004 11:41 am

Post by J.F. »

The PSP uses cooperative multitasking, so it's quite safe, despite no locking. There's no way on the PSP that threads can allocate memory "at the same time".
Post Reply