From d01d493a054dab5c95e946521ebfd6a4d1929122 Mon Sep 17 00:00:00 2001 From: Samier Merchant Date: Wed, 12 Feb 2025 20:54:01 -0500 Subject: [PATCH 1/4] Update ogrct.cpp to remove shared shared proj context (m_psLastContext), which can lead to scenarios where one thread destroys the context while another is still using it Undo https://github.com/OSGeo/gdal/commit/94ede75a44d62203bfb4b7d154d94427cacb1230, which causes segfaults when using gdal in a multi-threaded environment such as a thread worker pool. It might be the case that one thread "destroys" the context (m_psLastContext) while another thread is still using it, leading to a segfault. As you can see with the following line: m_psLastContext = OSRGetProjTLSContext(); --- ogr/ogrct.cpp | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/ogr/ogrct.cpp b/ogr/ogrct.cpp index ae93f7163579..d1310fa89dfd 100644 --- a/ogr/ogrct.cpp +++ b/ogr/ogrct.cpp @@ -21,7 +21,6 @@ #include #include #include -#include #include "cpl_conv.h" #include "cpl_error.h" @@ -750,9 +749,6 @@ class OGRProjCT : public OGRCoordinateTransformation double dfThreshold = 0.0; - PJ_CONTEXT *m_psLastContext = nullptr; - std::thread::id m_nLastContextThreadId{}; - PjPtr m_pj{}; bool m_bReversePj = false; @@ -1265,8 +1261,7 @@ OGRProjCT::OGRProjCT(const OGRProjCT &other) m_osTargetSRS(other.m_osTargetSRS), bWebMercatorToWGS84LongLat(other.bWebMercatorToWGS84LongLat), nErrorCount(other.nErrorCount), dfThreshold(other.dfThreshold), - m_psLastContext(nullptr), - m_nLastContextThreadId(std::this_thread::get_id()), m_pj(other.m_pj), + m_pj(other.m_pj), m_bReversePj(other.m_bReversePj), m_bEmitErrors(other.m_bEmitErrors), bNoTransform(other.bNoTransform), m_eStrategy(other.m_eStrategy), m_oTransformations(other.m_oTransformations), @@ -2520,14 +2515,7 @@ int OGRProjCT::TransformWithErrorCodes(size_t nCount, double *x, double *y, /* Select dynamically the best transformation for the data, if */ /* needed. */ /* -------------------------------------------------------------------- */ - PJ_CONTEXT *ctx = m_psLastContext; - const auto nThisThreadId = std::this_thread::get_id(); - if (!ctx || nThisThreadId != m_nLastContextThreadId) - { - m_nLastContextThreadId = nThisThreadId; - m_psLastContext = OSRGetProjTLSContext(); - ctx = m_psLastContext; - } + auto ctx = OSRGetProjTLSContext(); PJ *pj = m_pj; if (!bTransformDone && !pj) From f4ce0348a7749618e5f5d6e8f1652e1baa17fcf0 Mon Sep 17 00:00:00 2001 From: Alan Thomas Date: Sat, 15 Feb 2025 13:59:59 +1100 Subject: [PATCH 2/4] DXF: Don't hide block entities on layer 0 when that layer is frozen --- autotest/ogr/ogr_dxf.py | 33 ++++++++++++++++++++++++-- ogr/ogrsf_frmts/dxf/ogrdxf_feature.cpp | 18 +++++++------- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/autotest/ogr/ogr_dxf.py b/autotest/ogr/ogr_dxf.py index b07e71d3d5c3..cc61c92f8d9c 100644 --- a/autotest/ogr/ogr_dxf.py +++ b/autotest/ogr/ogr_dxf.py @@ -3881,7 +3881,7 @@ def test_ogr_dxf_53(): # Test frozen and off layers -def test_ogr_dxf_54(): +def test_ogr_dxf_54(tmp_vsimem): with gdal.config_option("DXF_MERGE_BLOCK_GEOMETRIES", "FALSE"): ds = ogr.Open("data/dxf/frozen-off.dxf") @@ -3897,7 +3897,36 @@ def test_ogr_dxf_54(): ) if isFeatureVisible == (h == "h"): f.DumpReadable() - pytest.fail("Wrong visibility on feature %d" % number) + pytest.fail( + "Wrong visibility on feature %d (testing with layer 0 thawed)" % number + ) + + # Rewrite the test file, this time with layer 0 set as frozen + with open("data/dxf/frozen-off.dxf", "r") as file: + gdal.FileFromMemBuffer( + tmp_vsimem / "frozen-off-with-layer0-frozen.dxf", + file.read().replace( + "0\nLAYER\n 2\n0\n 70\n 0", "0\nLAYER\n 2\n0\n 70\n 1" + ), + ) + + with gdal.config_option("DXF_MERGE_BLOCK_GEOMETRIES", "FALSE"): + ds = ogr.Open( + tmp_vsimem / "frozen-off-with-layer0-frozen.dxf", + ) + lyr = ds.GetLayer(0) + + # Repeat test - outcome should be the same + for number, h in enumerate(featureVisibility): + f = lyr.GetNextFeature() + isFeatureVisible = ( + "#000000)" in f.GetStyleString() or "#ff0000)" in f.GetStyleString() + ) + if isFeatureVisible == (h == "h"): + f.DumpReadable() + pytest.fail( + "Wrong visibility on feature %d (testing with layer 0 frozen)" % number + ) ############################################################################### diff --git a/ogr/ogrsf_frmts/dxf/ogrdxf_feature.cpp b/ogr/ogrsf_frmts/dxf/ogrdxf_feature.cpp index 8bb92b3aab50..36e395127f4e 100644 --- a/ogr/ogrsf_frmts/dxf/ogrdxf_feature.cpp +++ b/ogr/ogrsf_frmts/dxf/ogrdxf_feature.cpp @@ -146,9 +146,9 @@ OGRDXFFeature::GetColor(OGRDXFDataSource *const poDS, (poBlockFeature && poBlockFeature->oStyleProperties.count("Hidden") > 0)) { - // Hidden objects should never be shown no matter what happens, - // so they can be treated as if they are on a frozen layer - iHidden = 2; + // Hidden objects should never be shown no matter what happens + iHidden = 1; + oStyleProperties["Hidden"] = "1"; } else { @@ -166,13 +166,13 @@ OGRDXFFeature::GetColor(OGRDXFDataSource *const poDS, if (pszBlockHidden && atoi(pszBlockHidden) == 2) iHidden = 2; } - } - // If this feature is on a frozen layer, make the object totally - // hidden so it won't reappear if we regenerate the style string again - // during block insertion - if (iHidden == 2) - oStyleProperties["Hidden"] = "1"; + // If this feature is on a frozen layer (other than layer 0), make the + // object totally hidden so it won't reappear if we regenerate the style + // string again during block insertion + if (iHidden == 2 && !EQUAL(GetFieldAsString("Layer"), "0")) + oStyleProperties["Hidden"] = "1"; + } // Helpful constants const int C_BYLAYER = 256; From 504f825c9a6832d9074c472491cac71c9f687d3a Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Mon, 17 Feb 2025 19:33:59 +0100 Subject: [PATCH 3/4] Formatting fix --- ogr/ogrct.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ogr/ogrct.cpp b/ogr/ogrct.cpp index d1310fa89dfd..f04c993677c2 100644 --- a/ogr/ogrct.cpp +++ b/ogr/ogrct.cpp @@ -1261,9 +1261,9 @@ OGRProjCT::OGRProjCT(const OGRProjCT &other) m_osTargetSRS(other.m_osTargetSRS), bWebMercatorToWGS84LongLat(other.bWebMercatorToWGS84LongLat), nErrorCount(other.nErrorCount), dfThreshold(other.dfThreshold), - m_pj(other.m_pj), - m_bReversePj(other.m_bReversePj), m_bEmitErrors(other.m_bEmitErrors), - bNoTransform(other.bNoTransform), m_eStrategy(other.m_eStrategy), + m_pj(other.m_pj), m_bReversePj(other.m_bReversePj), + m_bEmitErrors(other.m_bEmitErrors), bNoTransform(other.bNoTransform), + m_eStrategy(other.m_eStrategy), m_oTransformations(other.m_oTransformations), m_iCurTransformation(other.m_iCurTransformation), m_options(other.m_options) From ff1fd90583e6044856f52ef127f32a14f67e2dfe Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Tue, 18 Feb 2025 11:59:46 +0100 Subject: [PATCH 4/4] CI alpine: add py3-numpy-tests package Cf https://gitlab.alpinelinux.org/alpine/aports/-/issues/16922 --- .github/workflows/alpine/Dockerfile.ci | 1 + .github/workflows/alpine_32bit/Dockerfile.ci | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/alpine/Dockerfile.ci b/.github/workflows/alpine/Dockerfile.ci index 291a18345ba0..1d77900b772b 100644 --- a/.github/workflows/alpine/Dockerfile.ci +++ b/.github/workflows/alpine/Dockerfile.ci @@ -57,6 +57,7 @@ RUN apk add \ py3-pyarrow-pyc \ py3-numpy \ py3-numpy-dev \ + py3-numpy-tests \ py3-pip \ py3-setuptools \ python3-dev \ diff --git a/.github/workflows/alpine_32bit/Dockerfile.ci b/.github/workflows/alpine_32bit/Dockerfile.ci index 935527a4a707..b16711eedee8 100644 --- a/.github/workflows/alpine_32bit/Dockerfile.ci +++ b/.github/workflows/alpine_32bit/Dockerfile.ci @@ -58,6 +58,7 @@ RUN apk add \ py3-pyarrow-pyc \ py3-numpy \ py3-numpy-dev \ + py3-numpy-tests \ py3-pip \ py3-setuptools \ python3-dev \