[Mapserver-dev] MS concurrency problem with vector symbols (partially solved)

[Mark Cave-Ayland]

This is a multi-part message in MIME format.

------=_NextPart_000_000C_01C32B68.4ED2D920
Content-Type: text/plain;
	charset="US-ASCII"
Content-Transfer-Encoding: 7bit

Hi Steve,

> -----Original Message-----
> From: mapserver-dev-admin at lists.gis.umn.edu 
> [mailto:mapserver-dev-admin at lists.gis.umn.edu] On Behalf Of 
> woodbri at swoodbridge.com
> Sent: 04 June 2003 23:28
> To: mapserver-dev at lists.gis.umn.edu; Mark Cave-Ayland
> Subject: Re: [Mapserver-dev] MS concurrency problem with 
> vector symbols
> 
> 
> Mark,
> 
> Mapserver, and therefore Mapscript, are not yet thread-safe. The 
> developers have been making a lot of changes to the source to remedy 
> this, and I don't think it is totally fixed in ms-3.7/4.0, let along 
> 3.6.5.
>
> You can search the archives for "thread safe" to get more info. By 
> the way, this is also the same reason that PHP must be built as a CGI 
> in Apache and not as a module if you want to use PHP/mapscript.

I don't know whether I am imagining things but I seem to recall that a
number of fixes went into the 3.6 branch a while back to deal with most
of these issues? I've just had a look back over the archives for the
past 6 months as you suggested, and the only outstanding issue I can
find relates to the FreeType library (paths aren't too much of a problem
as we use absolute paths for everything).

Anyway, I've spent several hours chipping away at this one and I've
managed to find where things were going wrong. The problem was that I
was calling setSymbolSet() which is not thread safe because it uses the
parser without locking. To fix the problem I implemented a lock around
msLoadSymbolSet() in mapsymbol.c and also had to change the lexer reset
code a little - please find the patch attached.

I've load tested this with 4000 requests over 10mins and it hasn't
failed once since the patch was applied. However.... This is not a great
fix because it defines a new lock type TLOCK_SUBPARSER to allow the call
to setSymbolSet() to work. Unfortunately this means that there is still
a conflict with TLOCK_PARSER since an entire mapfile could possibly be
lexed at the same time as the setSymbolSet() call is being executed
since the locks are different and I don't think it is possible to set
the same mutex twice without causing all sorts of problems :(.

I think what mapthread.c is missing is a routine msCheckLock(int) which
returns true if a lock has been taken. It would then be easy to modify
the patch so that if TLOCK_PARSER was not already set then it would be
set just for the duration of msLoadSymbolSet(). I've had a look at the
pthread documentation but there was no obvious way to implement this. It
would be great if the guy who wrote the code (Frank Warmerdam) could
suggest a way around this one.....


Many thanks,


Mark.


---

Mark Cave-Ayland
Webbased Ltd.
Tamar Science Park
Derriford
Plymouth
PL6 8BX
England

Tel: +44 (0)1752 764445
Fax: +44 (0)1752 764446


This email and any attachments are confidential to the intended
recipient and may also be privileged. If you are not the intended
recipient please delete it from your system and notify the sender. You
should not copy it or use it for any purpose nor disclose or distribute
its contents to any other person.

------=_NextPart_000_000C_01C32B68.4ED2D920
Content-Type: application/octet-stream;
	name="mapserver-3.6.5-setsymbol.patch"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
	filename="mapserver-3.6.5-setsymbol.patch"

--- mapserver-3.6.5.vanilla/mapsymbol.c	Wed Jun 26 14:12:40 2002=0A=
+++ mapserver-3.6.5/mapsymbol.c	Thu Jun  5 12:04:47 2003=0A=
@@ -3,6 +3,7 @@=0A=
 #include "map.h"=0A=
 #include "mapfile.h"=0A=
 #include "mapparser.h"=0A=
+#include "mapthread.h"=0A=
 =0A=
 extern int msyylex(); /* lexer globals */=0A=
 extern void msyyrestart();=0A=
@@ -11,6 +12,7 @@=0A=
 extern int msyylineno;=0A=
 extern FILE *msyyin;=0A=
 extern int msyyfiletype;=0A=
+extern int msyystate;=0A=
 =0A=
 extern unsigned char PNGsig[8];=0A=
 extern unsigned char JPEGsig[3];=0A=
@@ -440,6 +442,10 @@=0A=
   char *symbol_path;=0A=
   int status=3D1;=0A=
 =0A=
+=0A=
+  msAcquireLock( TLOCK_SUBPARSER );=0A=
+=0A=
+=0A=
   if(!symbolset) {=0A=
     msSetError(MS_SYMERR, "Symbol structure unallocated.", =
"msLoadSymbolFile()");=0A=
     return(-1);=0A=
@@ -460,7 +466,8 @@=0A=
   chdir(symbol_path);=0A=
   free(symbol_path);=0A=
 =0A=
-  msyylineno =3D 0; /* reset line counter */=0A=
+  msyystate =3D 5; // restore lexer state to INITIAL=0A=
+  msyylex();=0A=
   msyyrestart(msyyin); /* flush the scanner - there's a better way but =
this works for now */=0A=
   msyyfiletype =3D MS_FILE_SYMBOL;=0A=
 =0A=
@@ -496,6 +503,9 @@=0A=
   msyyfiletype =3D MS_FILE_DEFAULT;=0A=
   fclose(msyyin);=0A=
   chdir(old_path);=0A=
+=0A=
+  msReleaseLock( TLOCK_SUBPARSER );=0A=
+=0A=
   return(status);=0A=
 }=0A=
 =0A=
--- mapserver-3.6.5.vanilla/mapthread.h	Wed Jan  9 17:46:08 2002=0A=
+++ mapserver-3.6.5/mapthread.h	Thu Jun  5 12:03:07 2003=0A=
@@ -21,6 +21,7 @@=0A=
 #define TLOCK_GDAL	2=0A=
 #define TLOCK_ERROROBJ  3=0A=
 #define TLOCK_PROJ      4=0A=
+#define TLOCK_SUBPARSER	5=0A=
 =0A=
 #define TLOCK_STATIC_MAX 20=0A=
 #define TLOCK_MAX       100=0A=

------=_NextPart_000_000C_01C32B68.4ED2D920--