Skip to content

Commit 146d9db

Browse files
authored
Merge pull request #3 from PDAL/gil-management
manage GIL acquisition when doing things inside of pdal::plang
2 parents dce8ac1 + 99df2b9 commit 146d9db

File tree

9 files changed

+108
-13
lines changed

9 files changed

+108
-13
lines changed

.github/workflows/build.yml

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,21 @@ jobs:
2424
fail-fast: true
2525
matrix:
2626
os: ['ubuntu-latest', 'macos-latest', 'windows-latest']
27-
python-version: ['3.8', '3.9', '3.10']
27+
python-version: ['3.9', '3.10', '3.11', '3.12']
2828

2929
steps:
3030
- name: Check out
31-
uses: actions/checkout@v2
31+
uses: actions/checkout@v4
3232

3333
- name: Setup micromamba
34-
uses: mamba-org/provision-with-micromamba@main
34+
uses: conda-incubator/setup-miniconda@v3
3535
with:
36+
miniforge-variant: Mambaforge
37+
miniforge-version: latest
38+
use-mamba: true
39+
auto-update-conda: true
3640
environment-file: .github/environment.yml
37-
extra-specs: |
38-
python=${{ matrix.python-version }}
39-
sel(linux): compilers
41+
4042

4143
- name: Install
4244
shell: bash -l {0}
@@ -67,11 +69,14 @@ jobs:
6769
python-version: ['3.9']
6870

6971
steps:
70-
- uses: actions/checkout@v2
71-
- uses: conda-incubator/setup-miniconda@v2
72+
- uses: actions/checkout@v4
73+
- name: Setup micromamba
74+
uses: conda-incubator/setup-miniconda@v3
7275
with:
73-
channels: conda-forge
74-
python-version: ${{ matrix.python-version }}
76+
miniforge-variant: Mambaforge
77+
miniforge-version: latest
78+
use-mamba: true
79+
auto-update-conda: true
7580
mamba-version: "*"
7681

7782
- name: Dependencies
@@ -84,7 +89,7 @@ jobs:
8489
python setup.py sdist
8590
ls dist
8691
87-
- uses: pypa/gh-action-pypi-publish@master
92+
- uses: pypa/gh-action-pypi-publish@release/v1
8893
name: Publish package
8994
if: github.event_name == 'release' && github.event.action == 'published'
9095
with:

pdal/filters/PythonFilter.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ PointViewSet PythonFilter::run(PointViewPtr view)
148148
log()->get(LogLevel::Debug5) << "filters.python " << *m_script <<
149149
" processing " << (int)view->size() << " points." << std::endl;
150150

151+
plang::gil_scoped_acquire acquire;
151152
m_pythonMethod->execute(view, getMetadata());
152153

153154
PointViewSet viewSet;

pdal/io/NumpyReader.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ PyArrayObject* load_npy_script(std::string const& source,
198198
void NumpyReader::initialize()
199199
{
200200
plang::Environment::get();
201+
plang::gil_scoped_acquire acquire;
201202
m_numPoints = 0;
202203
m_chunkCount = 0;
203204
m_ndims = 0;
@@ -424,6 +425,7 @@ void NumpyReader::addDimensions(PointLayoutPtr layout)
424425
{
425426
using namespace Dimension;
426427

428+
plang::gil_scoped_acquire acquire;
427429
wakeUpNumpyArray();
428430
createFields(layout);
429431

@@ -485,6 +487,7 @@ void NumpyReader::addDimensions(PointLayoutPtr layout)
485487

486488
void NumpyReader::ready(PointTableRef table)
487489
{
490+
plang::gil_scoped_acquire acquire;
488491
plang::Environment::get()->set_stdout(log()->getLogStream());
489492

490493
// Set our iterators
@@ -528,6 +531,8 @@ bool NumpyReader::nextPoint()
528531
// just advance by the stride.
529532
if (--m_chunkCount == 0)
530533
{
534+
// Go grab the gil before we touch Python stuff again
535+
plang::gil_scoped_acquire acquire;
531536
// If we can't fetch the next ite
532537
if (!m_iternext(m_iter))
533538
return false;
@@ -653,6 +658,7 @@ bool NumpyReader::processOne(PointRef& point)
653658

654659
point_count_t NumpyReader::read(PointViewPtr view, point_count_t numToRead)
655660
{
661+
plang::gil_scoped_acquire acquire;
656662
PointId idx = view->size();
657663
point_count_t numRead(0);
658664

@@ -670,8 +676,8 @@ point_count_t NumpyReader::read(PointViewPtr view, point_count_t numToRead)
670676

671677
void NumpyReader::done(PointTableRef)
672678
{
679+
plang::gil_scoped_acquire acquire;
673680
// Dereference everything we're using
674-
675681
if (m_iter)
676682
NpyIter_Deallocate(m_iter);
677683

pdal/plang/Environment.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,19 @@ EnvironmentPtr Environment::get()
114114

115115
auto init = []()
116116
{
117+
PyGILState_STATE gstate;
118+
119+
// If the interpreter is already initialized, we need to
120+
// grab the GIL and hold it before we go do any Python
121+
// stuff
122+
bool alreadyInitialized(Py_IsInitialized());
123+
if (alreadyInitialized)
124+
gstate = PyGILState_Ensure();
125+
117126
g_environment = new Environment();
127+
128+
if (alreadyInitialized)
129+
PyGILState_Release(gstate);
118130
};
119131
std::call_once(flag, init);
120132
return g_environment;
@@ -150,8 +162,10 @@ Environment::Environment()
150162
throw pdal_error("unable to add redirector module!");
151163
}
152164

165+
153166
initNumpy();
154167
PyImport_ImportModule("redirector");
168+
155169
}
156170

157171
Environment::~Environment()

pdal/plang/Environment.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747

4848
#include "Redirector.hpp"
4949
#include "Script.hpp"
50+
#include "gil.hpp"
5051

5152
namespace pdal
5253
{

pdal/plang/Invocation.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ Invocation::Invocation(const Script& script, MetadataNode m,
122122
m_script(script), m_inputMetadata(m), m_pdalargs(pdalArgs)
123123
{
124124
Environment::get();
125+
gil_scoped_acquire acquire;
125126
compile();
126127
}
127128

@@ -357,6 +358,7 @@ PyObject* getPyJSON(std::string const& s)
357358
// Returns a new reference to a dictionary of numpy arrays/names.
358359
PyObject *Invocation::prepareData(PointViewPtr& view)
359360
{
361+
gil_scoped_acquire acquire;
360362
PointLayoutPtr layout(view->m_pointTable.layout());
361363
Dimension::IdList const& dims = layout->dims();
362364

pdal/plang/Redirector.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
// Blog article: http://mateusz.loskot.net/?p=2819
88

99
#include "Redirector.hpp"
10+
#include "gil.hpp"
1011

1112
#pragma warning(disable: 4127) // conditional expression is constant
1213
#pragma warning(disable: 4068) // gcc pragma warnings
@@ -30,6 +31,7 @@ struct Stdout
3031

3132
static PyObject* Stdout_write(PyObject* self, PyObject* args)
3233
{
34+
gil_scoped_acquire acquire;
3335
std::size_t written(0);
3436
Stdout* selfimpl = reinterpret_cast<Stdout*>(self);
3537
if (selfimpl->write)
@@ -48,6 +50,7 @@ static PyObject* Stdout_write(PyObject* self, PyObject* args)
4850

4951
static PyObject* Stdout_flush(PyObject* self, PyObject* /*args*/)
5052
{
53+
gil_scoped_acquire acquire;
5154
Stdout *selfimpl = reinterpret_cast<Stdout *>(self);
5255
if (selfimpl->flush)
5356
{
@@ -151,9 +154,12 @@ static struct PyModuleDef redirectordef = {
151154

152155
PyObject* Redirector::init()
153156
{
157+
gil_scoped_acquire acquire;
154158
StdoutType.tp_new = PyType_GenericNew;
155159
if (PyType_Ready(&StdoutType) < 0)
160+
{
156161
return NULL;
162+
}
157163
PyObject* m = PyModule_Create(&redirectordef);
158164
if (m)
159165
{
@@ -171,6 +177,8 @@ PyObject* Redirector::init()
171177

172178
void Redirector::set_stdout(std::ostream* ostr)
173179
{
180+
gil_scoped_acquire acquire;
181+
174182
stdout_write_type writeFunc = [ostr](std::string msg) { *ostr << msg; };
175183
stdout_flush_type flushFunc = [ostr]{ ostr->flush(); };
176184

@@ -180,6 +188,8 @@ void Redirector::set_stdout(std::ostream* ostr)
180188

181189
void Redirector::set_stdout(stdout_write_type write, stdout_flush_type flush)
182190
{
191+
gil_scoped_acquire acquire;
192+
183193
if (!m_stdout)
184194
{
185195
m_stdout_saved =
@@ -196,11 +206,15 @@ void Redirector::set_stdout(stdout_write_type write, stdout_flush_type flush)
196206

197207
void Redirector::reset_stdout()
198208
{
209+
PyGILState_STATE gstate;
210+
gstate = PyGILState_Ensure();
211+
199212
if (m_stdout_saved)
200213
PySys_SetObject(const_cast<char*>("stdout"), m_stdout_saved);
201214

202215
Py_XDECREF(m_stdout);
203216
m_stdout = 0;
217+
PyGILState_Release(gstate);
204218
}
205219

206220
} //namespace plang

pdal/plang/gil.hpp

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
pybind11/gil.h: RAII helpers for managing the GIL
3+
4+
Copyright (c) 2016 Wenzel Jakob <[email protected]>
5+
6+
All rights reserved. Use of this source code is governed by a
7+
BSD-style license that can be found in the LICENSE file.
8+
*/
9+
10+
#pragma once
11+
12+
#include <Python.h>
13+
14+
15+
namespace pdal
16+
{
17+
namespace plang
18+
{
19+
20+
21+
class gil_scoped_acquire {
22+
PyGILState_STATE state;
23+
24+
public:
25+
gil_scoped_acquire() : state{PyGILState_Ensure()} {}
26+
gil_scoped_acquire(const gil_scoped_acquire &) = delete;
27+
gil_scoped_acquire &operator=(const gil_scoped_acquire &) = delete;
28+
~gil_scoped_acquire() { PyGILState_Release(state); }
29+
void disarm() {}
30+
};
31+
32+
class gil_scoped_release {
33+
PyThreadState *state;
34+
35+
public:
36+
// PRECONDITION: The GIL must be held when this constructor is called.
37+
gil_scoped_release() {
38+
assert(PyGILState_Check());
39+
state = PyEval_SaveThread();
40+
}
41+
gil_scoped_release(const gil_scoped_release &) = delete;
42+
gil_scoped_release &operator=(const gil_scoped_release &) = delete;
43+
~gil_scoped_release() { PyEval_RestoreThread(state); }
44+
void disarm() {}
45+
};
46+
47+
48+
49+
50+
} // plang
51+
52+
} // pdal

setup.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
setup(
77
name="pdal-plugins",
8-
version="1.2.1",
8+
version="1.3.0",
99
description="Point cloud data processing Python plugins",
1010
license="BSD",
1111
keywords="point cloud spatial",

0 commit comments

Comments
 (0)