Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#3137 closed defect (fixed)

WFS layer: possible race condition

Reported by: assefa Owned by: dmorissette
Priority: normal Milestone: 6.0 release
Component: WFS Client Version: unspecified
Severity: normal Keywords:
Cc: jimk, assefa, aboudreault, sdlime

Description

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);

Change History (6)

comment:1 Changed 11 years ago by sdlime

Component: WMS ClientWFS Client

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

comment:2 Changed 11 years ago by dmorissette

Cc: assefa aboudreault sdlime added

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.

comment:3 Changed 11 years ago by dmorissette

Milestone: 5.6 release6.0 release
Owner: changed from mapserverbugs to dmorissette

comment:4 Changed 10 years ago by jimk

Resolution: fixed
Status: newclosed

Patch from DM applied in r9835.

comment:5 Changed 10 years ago by 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:

comment:6 Changed 10 years ago by dmorissette

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

Note: See TracTickets for help on using tickets.