Opened 11 years ago

Closed 11 years ago

#2229 closed defect (fixed)

The OGR VRT driver causes access violations with multiple threads

Reported by: tamas Owned by: warmerdam
Priority: normal Milestone: 1.5.1
Component: GDAL_Raster Version: svn-trunk
Severity: normal Keywords: GDALOpenShared
Cc:

Description

Currently the OGR VRT layer actually uses OpenShared? (ogrvrtlayer.cpp, line 176) to open the src datasource therefore the multiple threads will get the same datasource and layer to operate on. This is definitely unsave and the threads will access the memners of these objects simultaneously, causing access violations at random places. When I change the OpenShared? to Open the problem disappears.

Similar issue may exist in other parts of GDAL/OGR as well.

Change History (8)

comment:1 Changed 11 years ago by warmerdam

Component: OGR_SFGDAL_Raster
Keywords: GDALOpenShared added
Milestone: 1.5.1
Resolution: fixed
Status: newclosed

Tamas,

I have committed a change to shared dataset handling so that a dataset may not be shared between more than one thread (via GDALOpenShared()). Currently the change is only in trunk (r13805). Could you test and confirm is resolves your problem? Then we can back port to 1.5 branch.

....

Criminey, I see you were talking about OGR VRTs and VRT shared datasources, not GDAL where I fixed things. I'll try something similar in OGR in the morning.

Sorry for the mixup.

comment:2 Changed 11 years ago by tamas

Frank,

Yes, I'm interested in the OGR side primarily.

I'm a bit hesitant whether this option would provide a feasible solution in all cases. When the threads are managed in thread-pools the same thread can get executed at another time in a different application context. Using the same src-datasource and src-layer may eventually be unexpected by the application.

I'm tending to think that changing from OpenShared? to Open would be the most appropriate in the VRT layer initialization code. When the client wants to share this datasource he'll possibly use OpenShared? on the outer (VRT) datasource. This will result in that the src-layer will be shared among the clients holding reference to the same VRT datasource. Does it makes sense to share the src-datasource among the VRT layers when the VRT have been opened in a non-shared mode?

In addition we could also track the sharing mode of the VRT datasource and use the same mode when opening the source datasource if needed.

comment:3 Changed 11 years ago by warmerdam

Resolution: fixed
Status: closedreopened

comment:4 Changed 11 years ago by warmerdam

have applied a change in trunk (r13807) to track the PID (thread id) with OGR shared datasources, and only allow sharing within a thread. This avoids some sorts of problems but still leaves the issue that shared datasources have some state (attribute and spatial filters for instance) that can get applied to layers and affect other shared users unexpected. This patch can also *not* be back ported to 1.5 branch because it may change the ABI for the OGRSFDriverRegistrar class due to introduction of new member data in the class.

So, something else still needs

comment:5 Changed 11 years ago by warmerdam

Err,

So something else still needs to be done for VRT...

comment:6 Changed 11 years ago by warmerdam

I have applied changes to the VRT driver to:

  • check for a "shared" attribute on the SrcDataSource? element and use it to control sharing.
  • If not found, default shared to ON for SrcSQL layers since they are generally not going to impact other uses of the datasource as far as state goes.
  • If not found, default shared to OFF for SrcLayer? layers since they will tend to conflict with regard to feature iterator, and spatial/attribute filters.
  • VRT docs also updated.

I have not tested these changes well, so for now they are only in trunk (r13808) but they should be safe to port into 1.5 branch once tested.

comment:7 Changed 11 years ago by tamas

I've tested the fix against the original issue and it's working pretty well. Either the shared = ON or shared = OFF worked along with the SrcLayer? option, though I highly support the recommended default settings to use.

I think it's ready to be backported into 1.5 branch.

That's a great fix for the multithreading tribe of the applications.

Thanks

comment:8 Changed 11 years ago by warmerdam

Resolution: fixed
Status: reopenedclosed

I have back ported the change into 1.5 branch (r13812).

Note: See TracTickets for help on using tickets.