WFS layer: possible race condition

[assefa]

reported by jimk (as part of #3136)

I also found what appears to be a race condition in the creation of the temporary GML file. The problem occurs when bPostRequest is false and two similar requests occur at nearly the same time (in my case one with mode=map and an identical one but with mode=nquery&searchmap=true). Then both requests use the same filename for the temporary file which causes one of the two requests to fail. The problem code is in mapwfslayer.c lines 649-659.

Forcing the condition of the if statement to true eliminates the problem, but probably eliminates some caching efforts.

if (bPostRequest) // expect race condition between two requests causing second to fail with same URL if bPostRequest is false
    {
        char *pszPostTmpName = NULL;
        pszPostTmpName = (char *)malloc(sizeof(char)*(strlen(pszURL)+128));
        sprintf(pszPostTmpName,"%s%ld%d",
                pszURL, (long)time(NULL), (int)getpid());
        pszHashFileName = msHashString(pszPostTmpName);
        free(pszPostTmpName);
    }
    else
      pszHashFileName = msHashString(pszURL);

[sdlime]

Updating to correct component. Someone familiar with that code will have to comment on fixing before 5.6-beta4. -Steve

[dmorissette]

I think I should take the blame for this half-assed caching mechanism. I had a quick look and see no simple resolution, so I'd be tempted to remove this whole block and instead use msTmpFile() at line 663:

    pasReqInfo[(*numRequests)].pszOutputFile =  msTmpFile(map->mappath,
                                                          map->web.imagepath,
                                                          ".tmp.gml");

This would be in line with what mapwmslayer.c does for WMS client layers.

Then we should file a enhancement ticket for 6.0 to look into implementing some real/smart caching for WMS and WFS client layers in general, and that works for both GET and POST requests.

I don't dare to make this change and commit it now just before leaving, but if one of you has time to test then please go ahead.

[jimk]

Patch from DM applied in r9835.

[tomkralidis]

Jim: FYI I get a build warning on trunk (fc11):

mapwfslayer.c: In function 'msPrepareWFSLayerRequest':
mapwfslayer.c:517: warning: unused variable 'pszHashFileName'
mapwfslayer.c: At top level:

[dmorissette]

Backported fix to SVN branch-5-6 in r10264 (will be in 5.6.4)