Bug in msCopy

[Umberto Nicoletti]

On 6/29/05, Benedikt Rothe <umn-ms at hydrotec.de> wrote:
>  
> Dear List 
>  
> We are using MapScript/Tomcat/Java/OracleSpatial. 
>  
> We use mapObj.mapClone() to get a fresh mapObject 
> in each request without reading the Mapfile. 

Benedikt,
I would not do that (tm). In my apps I recreate a fresh mapObj with
each request  as the overhead is mininum and the benefits high (so
that the ratio is in favor of this approach).
Besides this is the only known method to work (as far as I know) and
this stems from the fact that mapserver was in the beginning (and
still is, with improvements) a CGI. As you might know cgis are
processes that are created by the web server to serve a single request
and then exit, so memory cleanup and thread safety are non-issues for
them.
Even in php mapscript php must be used as a cgi (instead of a DSO) to
make sure that it works.

>  
> We detected problems after we activated Connection-Pooling: 
> Tomcat crashed in mapObj.delete() 
>

I would not use connection pooling just because it will not work
reliably with Java Servlets for the reasons explained above.

This matter (connection pooling and map cloning) surely requires
deeper investigation, one of the developers coul step in to give his
opinion on my statements (which I would sincerely hope were false, but
fear not). Btw, I seem to recall that the above issues (long lived
instance against per request instance)  were already discussed on the
list before.

Best regards,
Umberto
  
> Hole thing turned out to be a bug in mapcopy.c where 
> the joins are copied: 
> -------------------- 
> MS_COPYSTELEM(numjoins); 
>  
> for (i = 0; i < dst->numprocessing; i++) { 
>    return_value = msCopyJoin(&(dst->joins[i]), &(src->joins[i])); 
> -------------------- 
>  
> This must be changed to 
> -------------------- 
> MS_COPYSTELEM(numjoins); 
>  
> for (i = 0; i < dst->numjoins; i++) { 
>    return_value = msCopyJoin(&(dst->joins[i]), &(src->joins[i])); 
> -------------------- 
>  
> There is some other code in mapobject.c, wich didn't 
> raise any problems for us, but from wich I do think 
> that array-allocs are missing. 
> I watched the code an *think* these are are not allocated 
> anywhere. I hope, there is no misleading hint: 
> 
>  ----------------------------
>  // I do think, that dst->point is not allocated.
>  // code seems to be unused (?)
>  dst->numpoints = src->numpoints;
>  for (i = 0; i < dst->numpoints; i++) {
>   MS_COPYPOINT(&(dst->point[i]), &(src->point[i]));
>  }
>  ----------------------------
>  // I do think, that dst->items and dst->values is
>  // not allocated
>  MS_COPYSTELEM(numitems);
>  for (i = 0; i < dst->numitems; i++) {
>   MS_COPYSTRING(dst->items[i], src->items[i]);
>   MS_COPYSTRING(dst->values[i], src->values[i]);
>  }
>  
>  -----------------------------
>  // I'm quite shure, that dst->items is not allocated
>  MS_COPYSTELEM(numitems);
>  
>  for (i = 0; i < dst->numitems; i++) {
>   MS_COPYSTRING(dst->items[i], src->items[i]);
>  }
>  
> I hope this is helpful. Greetings 
> Benedikt Rothe