ID: 40858 User updated by: scottmacvicar at ntlworld dot com Reported By: scottmacvicar at ntlworld dot com Status: Assigned Bug Type: GD related Operating System: RHEL 4 PHP Version: 5.2.1 Assigned To: pajoye New Comment:
I don't have access to a Windows machine at the moment but the following patch should fix the issues in libGD, I'll try and do a test when I have access to the machine. PHP 5.2 Patch: http://public.vbulletin.com/bugs/php/gd-mutex-patch.txt libGD Patch: http://public.vbulletin.com/bugs/php/libgd-mutex-patch.txt Explanation: * pthread now initialises the mutex only once * win32 can set the mutex within DLLMain * Race condition fix by locking prior to checking font cache Problems not fixed: * calling gdFontCacheSetup before obtaining a lock is going to cause the race condition still, I know the ruby wrappers do this. * Unusure about behaviour of using the non bundled GD library on windows with PHP since DLLMain and gdFontCacheMutexSetup may setup the critical section twice. Could make gdMutexSetup(x) a no-op for win32 too. Previous Comments: ------------------------------------------------------------------------ [2007-03-23 01:21:26] [EMAIL PROTECTED] Hi Scott, Nuno, "Regarding the patch to the bundled GD library and the php wrapper, does it look reasonable? We've been running it for the past 2 days now without issue on a production server. I'd rather see something put into 5.2.2 " Yes, the patch for php (and unix) looks good. I will apply it during the weekend as well (or Nuno, you can do it if you have the time before). For the windows version, that's a good news, thanks to take the time to work on it. ------------------------------------------------------------------------ [2007-03-23 00:31:35] scottmacvicar at ntlworld dot com I have some time to spare over the weekend and a copy of VC6 installed now so I'll have a go at the win32 implementation. Regarding the patch to the bundled GD library and the php wrapper, does it look reasonable? We've been running it for the past 2 days now without issue on a production server. I'd rather see something put into 5.2.2 and then fix libGD. ------------------------------------------------------------------------ [2007-03-22 23:15:13] [EMAIL PROTECTED] yep, having a statically declared mutex would fix the problems we are having. I've a patch identical to yours (http://mega.ist.utl.pt/~ncpl/libgd_mutexes.txt) for a while, but we are still waiting for a win32 implementation to merge the patches (there is another open bug report in the gd bugs db about a similar issue) to definitely fix those nasty TS issues. ------------------------------------------------------------------------ [2007-03-21 01:37:18] scottmacvicar at ntlworld dot com Patch is available at http://public.vbulletin.com/bugs/php/gd-mutex-patch.txt Quick explanation: * The mutex is created at module startup and destroyed at module shutdown courtesy of two new helper functions within the libGD code base. * The mutex is no longer destroyed at request shutdown. * Locking is performed prior to checking fontCache to prevent the race condition we're seeing. This can't be merged back into libGD at the moment since it breaks backwards compatibility by assuming a mutex has been allocated, the way to fix this for pthread is fairly easy. #define gdMutexDeclare(x) pthread_mutex_t x #define gdMutexDeclare(x) pthread_mutex_t x becomes #define gdMutexDeclare(x) static pthread_mutex_t x = PTHREAD_MUTEX_INITIALIZER #define gdMutexSetup(x) That would just leave the win32 implementation to deal with via DLLMain, a quick patch to add that would only take a few minutes though I don't have VC6 installed yet. ------------------------------------------------------------------------ [2007-03-21 00:48:56] [EMAIL PROTECTED] "Moving the code to PHP_MINIT fixes the problem for PHP but the threading options of GD doesn't extend itself to this setup since the function to create the mutex can be called more than once in a short period of time." Do you mean for libGD itself used a library in a random project? If yes, a solution is very similar to this one. We have to do it at load time (like DLLMain on windows). Many other libraries (and many related to FT2, with ft2 cache or their own) rely on this solution. We can give it a try, at least. "I've got a patch that implements the PHP solution running on our boxes, I can provide it if interested." You can always post it, I still did not get the time to do it, so any help is welcome. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://bugs.php.net/40858 -- Edit this bug report at http://bugs.php.net/?id=40858&edit=1
