Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to render thread-safe the package, with test. #69

Merged
merged 19 commits into from
Jun 11, 2024
Merged

Changes to render thread-safe the package, with test. #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d130384
Changed string_pointer into standard string* and changed new() to be …
fpl Nov 21, 2023
52559f5
Changed t/threads.t to skip when ithreads are not implemented
fpl Nov 21, 2023
5660700
vsistdout.t: write method needs to return 1
shawnlaffan Nov 18, 2023
20325bb
t/vsistdout.t: decode JSON and use is_deeply for test
shawnlaffan Nov 18, 2023
99ce608
Release version 0.11
Dec 19, 2023
889a34c
Set version to 0.12
Dec 19, 2023
2453e18
This branch closes #53 and render Geo::GDAL::FFI thread-safe by disab…
fpl Dec 19, 2023
b727cd7
Added a note about thread-safety and use of SetErrorHandling() within…
fpl Jan 4, 2024
87a5d6c
Removed note in Changes to avoid conflicts with ajolma/master.
fpl Jan 4, 2024
76ac0e3
Updated for typos in comment and POD documentation.
fpl Jun 11, 2024
a685e77
use strict and warnings in Makefile.PL
shawnlaffan Dec 19, 2023
c1841c3
use strict and warnings in dataset_rasterize.t
shawnlaffan Dec 19, 2023
3c0a10a
Tests: use Test::TempDir::Tiny and Path::Tiny
shawnlaffan Dec 19, 2023
f95d774
CI: install more CPAN deps
shawnlaffan Dec 19, 2023
99c76f2
CI: update actions versions
shawnlaffan Jun 11, 2024
1bfc096
CI: Update perl version to 5.38
shawnlaffan Jun 11, 2024
f344fcf
CI: install deps on linux bin build
shawnlaffan Jun 11, 2024
1902641
Avoid some used-once warnings
shawnlaffan Oct 29, 2023
d5a06af
Merge branch 'master' into new_thread-safe
shawnlaffan Jun 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions build-tools/parse_h.pl
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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();
Loading