Skip to content

Commit

Permalink
Merge pull request #69 from fpl/new_thread-safe
Browse files Browse the repository at this point in the history
Changes to render thread-safe the package, with test.
  • Loading branch information
shawnlaffan authored Jun 11, 2024
2 parents 1c144d7 + d5a06af commit 1d6fa91
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 33 deletions.
4 changes: 2 additions & 2 deletions build-tools/parse_h.pl
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,8 @@ sub parse_type {
$arg = 'opaque';
} else {
my $ok = $char_p_p_ok{$name} || $char_p_p_ok{$name.'.'.$var};
say STDERR "WARNING: char ** in $name ($mode, $var) defaulting to string_pointer" unless $ok;
$arg = 'string_pointer';
say STDERR "$name returns a string*" unless $ok;
$arg = 'string*';
}
} elsif ($arg =~ /char\s*\*/) {
if ($mode eq 'ret' && $use_ret_opaque{$name}) {
Expand Down
89 changes: 58 additions & 31 deletions lib/Geo/GDAL/FFI.pm
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,14 @@ sub SetErrorHandling {
push @errors, $msg;
}
});
$instance->{CPLErrorHandler}->sticky;
CPLPushErrorHandler($instance->{CPLErrorHandler});
}

sub UnsetErrorHandling {
return unless $instance;
return unless exists $instance->{CPLErrorHandler};
$instance->{CPLErrorHandler}->unstick;
CPLPopErrorHandler($instance->{CPLErrorHandler});
delete $instance->{CPLErrorHandler};
}
Expand Down Expand Up @@ -436,7 +438,6 @@ sub new {
return $instance if $instance;

my $ffi = FFI::Platypus->new;
$ffi->load_custom_type('::StringPointer' => 'string_pointer');
my @libs = $gdal->dynamic_libs;
$ffi->lib(@libs);

Expand All @@ -460,13 +461,14 @@ sub new {
$ffi->attach(VSIFCloseL => ['opaque'] => 'int');
$ffi->attach(VSIFWriteL => [qw/opaque uint uint opaque/] => 'uint');
$ffi->attach(VSIFReadL => [qw/opaque uint uint opaque/] => 'uint');
$ffi->attach(VSIIngestFile => [qw/opaque string string_pointer uint64* sint64/] => 'int');
$ffi->attach(VSIIngestFile => [qw/opaque string string* uint64* sint64/] => 'int');
$ffi->attach(VSIMkdir => [qw/string sint64/] => 'int');
$ffi->attach(VSIRmdir => [qw/string/] => 'int');
$ffi->attach(VSIReadDirEx => [qw/string int/] => 'opaque');
$ffi->attach(VSIUnlink => [qw/string/] => 'int');
$ffi->attach(VSIRename => [qw/string string/] => 'int');
$ffi->attach(VSIStdoutSetRedirection => ['VSIWriteFunction', 'opaque'] => 'void');
$ffi->attach(CPLSetErrorHandler => ['CPLErrorHandler'] => 'opaque');
$ffi->attach(CPLPushErrorHandler => ['CPLErrorHandler'] => 'void');
$ffi->attach(CPLPopErrorHandler => ['CPLErrorHandler'] => 'void');
$ffi->attach(CSLDestroy => ['opaque'] => 'void');
Expand Down Expand Up @@ -560,7 +562,7 @@ $ffi->attach('GDALDatasetRasterIO' => ['opaque','unsigned int','int','int','int'
$ffi->attach('GDALDatasetRasterIOEx' => ['opaque','unsigned int','int','int','int','int','opaque','int','int','unsigned int','int','int*','sint64','sint64','sint64','opaque'] => 'int');
$ffi->attach('GDALDatasetAdviseRead' => ['opaque','int','int','int','int','int','int','unsigned int','int','int*','opaque'] => 'int');
$ffi->attach('GDALDatasetGetCompressionFormats' => [qw/opaque int int int int int int*/] => 'opaque');
$ffi->attach('GDALDatasetReadCompressedData' => [qw/opaque string int int int int int int* opaque size_t string_pointer/] => 'int');
$ffi->attach('GDALDatasetReadCompressedData' => [qw/opaque string int int int int int int* opaque size_t string*/] => 'int');
$ffi->attach('GDALGetProjectionRef' => [qw/opaque/] => 'string');
$ffi->attach('GDALGetSpatialRef' => [qw/opaque/] => 'opaque');
$ffi->attach('GDALSetProjection' => [qw/opaque string/] => 'int');
Expand Down Expand Up @@ -609,14 +611,14 @@ $ffi->attach('GDALDatasetRollbackTransaction' => [qw/opaque/] => 'int');
$ffi->attach('GDALDatasetClearStatistics' => [qw/opaque/] => 'void');
$ffi->attach('GDALDatasetGetFieldDomainNames' => [qw/opaque opaque/] => 'opaque');
$ffi->attach('GDALDatasetGetFieldDomain' => [qw/opaque string/] => 'opaque');
$ffi->attach('GDALDatasetAddFieldDomain' => [qw/opaque opaque string_pointer/] => 'bool');
$ffi->attach('GDALDatasetDeleteFieldDomain' => [qw/opaque string string_pointer/] => 'bool');
$ffi->attach('GDALDatasetUpdateFieldDomain' => [qw/opaque opaque string_pointer/] => 'bool');
$ffi->attach('GDALDatasetAddFieldDomain' => [qw/opaque opaque string*/] => 'bool');
$ffi->attach('GDALDatasetDeleteFieldDomain' => [qw/opaque string string*/] => 'bool');
$ffi->attach('GDALDatasetUpdateFieldDomain' => [qw/opaque opaque string*/] => 'bool');
$ffi->attach('GDALDatasetGetRelationshipNames' => [qw/opaque opaque/] => 'opaque');
$ffi->attach('GDALDatasetGetRelationship' => [qw/opaque string/] => 'opaque');
$ffi->attach('GDALDatasetAddRelationship' => [qw/opaque opaque string_pointer/] => 'bool');
$ffi->attach('GDALDatasetDeleteRelationship' => [qw/opaque string string_pointer/] => 'bool');
$ffi->attach('GDALDatasetUpdateRelationship' => [qw/opaque opaque string_pointer/] => 'bool');
$ffi->attach('GDALDatasetAddRelationship' => [qw/opaque opaque string*/] => 'bool');
$ffi->attach('GDALDatasetDeleteRelationship' => [qw/opaque string string*/] => 'bool');
$ffi->attach('GDALDatasetUpdateRelationship' => [qw/opaque opaque string*/] => 'bool');
$ffi->attach('GDALDatasetSetQueryLoggerFunc' => [qw/opaque GDALQueryLoggerFunc opaque/] => 'bool');
$ffi->attach('GDALGetRasterDataType' => [qw/opaque/] => 'unsigned int');
$ffi->attach('GDALGetBlockSize' => [qw/opaque int* int*/] => 'void');
Expand Down Expand Up @@ -685,7 +687,7 @@ $ffi->attach('GDALGetDataCoverageStatus' => [qw/opaque int int int int int doubl
$ffi->attach('GDALARGetNextUpdatedRegion' => [qw/opaque double int* int* int* int*/] => 'unsigned int');
$ffi->attach('GDALARLockBuffer' => [qw/opaque double/] => 'int');
$ffi->attach('GDALARUnlockBuffer' => [qw/opaque/] => 'void');
$ffi->attach('GDALGeneralCmdLineProcessor' => [qw/int string_pointer int/] => 'int');
$ffi->attach('GDALGeneralCmdLineProcessor' => [qw/int string* int/] => 'int');
$ffi->attach('GDALSwapWords' => [qw/opaque int int int/] => 'void');
$ffi->attach('GDALSwapWordsEx' => [qw/opaque int size_t int/] => 'void');
$ffi->attach('GDALCopyWords' => ['opaque','unsigned int','int','opaque','unsigned int','int','int'] => 'void');
Expand All @@ -695,10 +697,10 @@ $ffi->attach('GDALDeinterleave' => ['opaque','unsigned int','int','opaque','unsi
$ffi->attach('GDALLoadWorldFile' => [qw/string double*/] => 'int');
$ffi->attach('GDALReadWorldFile' => [qw/string string double*/] => 'int');
$ffi->attach('GDALWriteWorldFile' => [qw/string string double*/] => 'int');
$ffi->attach('GDALLoadTabFile' => [qw/string double* string_pointer int* opaque/] => 'int');
$ffi->attach('GDALReadTabFile' => [qw/string double* string_pointer int* opaque/] => 'int');
$ffi->attach('GDALLoadOziMapFile' => [qw/string double* string_pointer int* opaque/] => 'int');
$ffi->attach('GDALReadOziMapFile' => [qw/string double* string_pointer int* opaque/] => 'int');
$ffi->attach('GDALLoadTabFile' => [qw/string double* string* int* opaque/] => 'int');
$ffi->attach('GDALReadTabFile' => [qw/string double* string* int* opaque/] => 'int');
$ffi->attach('GDALLoadOziMapFile' => [qw/string double* string* int* opaque/] => 'int');
$ffi->attach('GDALReadOziMapFile' => [qw/string double* string* int* opaque/] => 'int');
$ffi->attach('GDALDecToDMS' => [qw/double string int/] => 'string');
$ffi->attach('GDALPackedDMSToDec' => [qw/double/] => 'double');
$ffi->attach('GDALDecToPackedDMS' => [qw/double/] => 'double');
Expand Down Expand Up @@ -911,7 +913,7 @@ $ffi->attach('GDALDimensionSetIndexingVariable' => [qw/opaque opaque/] => 'int')
$ffi->attach('OGRGetGEOSVersion' => [qw/int* int* int*/] => 'bool');
$ffi->attach('OGR_G_CreateFromWkb' => [qw/string opaque uint64* int/] => 'int');
$ffi->attach('OGR_G_CreateFromWkbEx' => [qw/opaque opaque uint64* size_t/] => 'int');
$ffi->attach('OGR_G_CreateFromWkt' => [qw/string_pointer opaque uint64*/] => 'int');
$ffi->attach('OGR_G_CreateFromWkt' => [qw/string* opaque uint64*/] => 'int');
$ffi->attach('OGR_G_CreateFromFgf' => [qw/string opaque uint64* int int*/] => 'int');
$ffi->attach('OGR_G_DestroyGeometry' => [qw/opaque/] => 'void');
$ffi->attach('OGR_G_CreateGeometry' => ['unsigned int'] => 'opaque');
Expand Down Expand Up @@ -939,9 +941,9 @@ $ffi->attach('OGR_G_ExportToWkb' => ['opaque','unsigned int','string'] => 'int')
$ffi->attach('OGR_G_ExportToIsoWkb' => ['opaque','unsigned int','string'] => 'int');
$ffi->attach('OGR_G_WkbSize' => [qw/opaque/] => 'int');
$ffi->attach('OGR_G_WkbSizeEx' => [qw/opaque/] => 'size_t');
$ffi->attach('OGR_G_ImportFromWkt' => [qw/opaque string_pointer/] => 'int');
$ffi->attach('OGR_G_ExportToWkt' => [qw/opaque string_pointer/] => 'int');
$ffi->attach('OGR_G_ExportToIsoWkt' => [qw/opaque string_pointer/] => 'int');
$ffi->attach('OGR_G_ImportFromWkt' => [qw/opaque string*/] => 'int');
$ffi->attach('OGR_G_ExportToWkt' => [qw/opaque string*/] => 'int');
$ffi->attach('OGR_G_ExportToIsoWkt' => [qw/opaque string*/] => 'int');
$ffi->attach('OGR_G_GetGeometryType' => [qw/opaque/] => 'unsigned int');
$ffi->attach('OGR_G_GetGeometryName' => [qw/opaque/] => 'string');
$ffi->attach('OGR_G_DumpReadable' => [qw/opaque opaque string/] => 'void');
Expand Down Expand Up @@ -1348,7 +1350,7 @@ $ffi->attach('OSRRelease' => [qw/opaque/] => 'void');
$ffi->attach('OSRValidate' => [qw/opaque/] => 'int');
$ffi->attach('OSRImportFromEPSG' => [qw/opaque int/] => 'int');
$ffi->attach('OSRImportFromEPSGA' => [qw/opaque int/] => 'int');
$ffi->attach('OSRImportFromWkt' => [qw/opaque string_pointer/] => 'int');
$ffi->attach('OSRImportFromWkt' => [qw/opaque string*/] => 'int');
$ffi->attach('OSRImportFromProj4' => [qw/opaque string/] => 'int');
$ffi->attach('OSRImportFromESRI' => [qw/opaque opaque/] => 'int');
$ffi->attach('OSRImportFromPCI' => [qw/opaque string string double*/] => 'int');
Expand All @@ -1360,16 +1362,16 @@ $ffi->attach('OSRImportFromOzi' => [qw/opaque opaque/] => 'int');
$ffi->attach('OSRImportFromMICoordSys' => [qw/opaque string/] => 'int');
$ffi->attach('OSRImportFromERM' => [qw/opaque string string string/] => 'int');
$ffi->attach('OSRImportFromUrl' => [qw/opaque string/] => 'int');
$ffi->attach('OSRExportToWkt' => [qw/opaque string_pointer/] => 'int');
$ffi->attach('OSRExportToWktEx' => [qw/opaque string_pointer opaque/] => 'int');
$ffi->attach('OSRExportToPrettyWkt' => [qw/opaque string_pointer int/] => 'int');
$ffi->attach('OSRExportToPROJJSON' => [qw/opaque string_pointer opaque/] => 'int');
$ffi->attach('OSRExportToProj4' => [qw/opaque string_pointer/] => 'int');
$ffi->attach('OSRExportToPCI' => [qw/opaque string_pointer string_pointer double*/] => 'int');
$ffi->attach('OSRExportToWkt' => [qw/opaque string*/] => 'int');
$ffi->attach('OSRExportToWktEx' => [qw/opaque string* opaque/] => 'int');
$ffi->attach('OSRExportToPrettyWkt' => [qw/opaque string* int/] => 'int');
$ffi->attach('OSRExportToPROJJSON' => [qw/opaque string* opaque/] => 'int');
$ffi->attach('OSRExportToProj4' => [qw/opaque string*/] => 'int');
$ffi->attach('OSRExportToPCI' => [qw/opaque string* string* double*/] => 'int');
$ffi->attach('OSRExportToUSGS' => [qw/opaque long* long* double* long*/] => 'int');
$ffi->attach('OSRExportToXML' => [qw/opaque string_pointer string/] => 'int');
$ffi->attach('OSRExportToXML' => [qw/opaque string* string/] => 'int');
$ffi->attach('OSRExportToPanorama' => [qw/opaque long* long* long* long* double*/] => 'int');
$ffi->attach('OSRExportToMICoordSys' => [qw/opaque string_pointer/] => 'int');
$ffi->attach('OSRExportToMICoordSys' => [qw/opaque string*/] => 'int');
$ffi->attach('OSRExportToERM' => [qw/opaque string string string/] => 'int');
$ffi->attach('OSRMorphToESRI' => [qw/opaque/] => 'int');
$ffi->attach('OSRMorphFromESRI' => [qw/opaque/] => 'int');
Expand All @@ -1379,13 +1381,13 @@ $ffi->attach('OSRGetName' => [qw/opaque/] => 'string');
$ffi->attach('OSRSetAttrValue' => [qw/opaque string string/] => 'int');
$ffi->attach('OSRGetAttrValue' => [qw/opaque string int/] => 'string');
$ffi->attach('OSRSetAngularUnits' => [qw/opaque string double/] => 'int');
$ffi->attach('OSRGetAngularUnits' => [qw/opaque string_pointer/] => 'double');
$ffi->attach('OSRGetAngularUnits' => [qw/opaque string*/] => 'double');
$ffi->attach('OSRSetLinearUnits' => [qw/opaque string double/] => 'int');
$ffi->attach('OSRSetTargetLinearUnits' => [qw/opaque string string double/] => 'int');
$ffi->attach('OSRSetLinearUnitsAndUpdateParameters' => [qw/opaque string double/] => 'int');
$ffi->attach('OSRGetLinearUnits' => [qw/opaque string_pointer/] => 'double');
$ffi->attach('OSRGetTargetLinearUnits' => [qw/opaque string string_pointer/] => 'double');
$ffi->attach('OSRGetPrimeMeridian' => [qw/opaque string_pointer/] => 'double');
$ffi->attach('OSRGetLinearUnits' => [qw/opaque string*/] => 'double');
$ffi->attach('OSRGetTargetLinearUnits' => [qw/opaque string string*/] => 'double');
$ffi->attach('OSRGetPrimeMeridian' => [qw/opaque string*/] => 'double');
$ffi->attach('OSRIsGeographic' => [qw/opaque/] => 'int');
$ffi->attach('OSRIsDerivedGeographic' => [qw/opaque/] => 'int');
$ffi->attach('OSRIsLocal' => [qw/opaque/] => 'int');
Expand Down Expand Up @@ -1826,6 +1828,21 @@ BEGIN {
local $FFI::Platypus::TypeParser::ffi_type;
}

#
# The next two subs are required for thread-safety, because GDAL error handling must be set per thread.
# So, it is disabled just before starting a new thread and renabled after in the thread.
# See perlmod and issue #53 for more information.
#

sub CLONE {
SetErrorHandling();
}

sub CLONE_SKIP {
UnsetErrorHandling();
return 0;
}

1;

=pod
Expand Down Expand Up @@ -2051,6 +2068,16 @@ confessed if a method fails. This is the default.
Unset the Perl function to catch GDAL errors. If no other error
handler is set, GDAL prints the errors into stderr.
=head1 NOTES ABOUT THREAD-SAFETY
This module is thread-safe provided the error handling is taken care of.
To ensure thread-safety GDAL error handling is automatically disabled
before creating a new thread and re-enabled after that in the just
created thread. The main thread needs to renable it via C<SetErrorHandling>,
after all thread creations and before eventually using any GDAL function. This
must be done explicitly in the main thread because there is no way
to do that automatically as for other threads.
=head1 METHODS
=head2 get_instance
Expand Down
63 changes: 63 additions & 0 deletions t/threads.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
use v5.18;
use warnings;
use strict;
use Config;
use Test::More;

BEGIN {
use_ok('Geo::GDAL::FFI', qw/:all/);
}

SKIP: {

skip "skip multi-thread test", 4 unless $Config{useithreads};

use_ok('threads');
use_ok('threads::shared');
use_ok('Thread::Queue');

my $q = Thread::Queue->new();
my @in_thrds = ();
my @out_thrds = ();
my $nt = 10;

for my $i (1..$nt) {
my $t = threads->create(
sub {
while (my $h = $q->dequeue()) {
say "thread out$i: popped $h->{value}";
}
}
);
push @out_thrds, $t;
}

for my $i (1..$nt) {
my $t = threads->create(
sub {
my $v = rand(100);
say "thread in$i: pushed $v";
$q->enqueue({ value => $v });
}
);
push @in_thrds, $t;
}

# try different timing too... :-/
sleep(3);

$q->end();

for my $w (@in_thrds) {
$w->join();
}

for my $w (@out_thrds) {
$w->join();
}

ok(1, "threading seems ok");

}

done_testing();

0 comments on commit 1d6fa91

Please sign in to comment.