From 1205024e2b9e69ee10ab27c0168ff987a8245c88 Mon Sep 17 00:00:00 2001
From: Andreas Kloeckner <inform@tiker.net>
Date: Tue, 10 May 2022 16:02:00 -0500
Subject: [PATCH] Warn/error if queue used after context manager exit
---
doc/runtime_queue.rst | 9 +++++-
pyopencl/__init__.py | 5 ++++
src/wrap_cl.hpp | 65 +++++++++++++++++++++++++++++++++---------
src/wrap_cl_part_1.cpp | 1 +
test/test_wrapper.py | 9 ++++++
5 files changed, 75 insertions(+), 14 deletions(-)
diff --git a/doc/runtime_queue.rst b/doc/runtime_queue.rst
index f120c61e..fbe0b664 100644
--- a/doc/runtime_queue.rst
+++ b/doc/runtime_queue.rst
@@ -32,7 +32,7 @@ Command Queue
enqueue_stuff(queue, ...)
:meth:`finish` is automatically called at the end of the ``with``-delimited
- context.
+ context, and further operations on the queue are considered an error.
.. versionadded:: 2013.1
@@ -42,6 +42,13 @@ Command Queue
Added the sequence-of-properties interface for OpenCL 2.
+ .. versionchanged:: 2022.1.4
+
+ Use of a command queue after its context manager completes
+ is now considered an error. :mod:`pyopencl` will warn about this
+ for a transitionary period and will start raising an exception
+ in 2023.
+
.. attribute:: info
Lower case versions of the :class:`command_queue_info` constants
diff --git a/pyopencl/__init__.py b/pyopencl/__init__.py
index a72f66b9..3f8dc83d 100644
--- a/pyopencl/__init__.py
+++ b/pyopencl/__init__.py
@@ -261,6 +261,10 @@ class CompilerWarning(UserWarning):
pass
+class CommandQueueUsedAfterExit(UserWarning):
+ pass
+
+
def compiler_output(text):
import os
from warnings import warn
@@ -706,6 +710,7 @@ def _add_functionality():
def command_queue_exit(self, exc_type, exc_val, exc_tb):
self.finish()
+ self._finalize()
def command_queue_get_cl_version(self):
return self.device._get_cl_version()
diff --git a/src/wrap_cl.hpp b/src/wrap_cl.hpp
index 4c40e373..63500c84 100644
--- a/src/wrap_cl.hpp
+++ b/src/wrap_cl.hpp
@@ -1378,17 +1378,26 @@ namespace pyopencl
{
private:
cl_command_queue m_queue;
+ // m_finalized==True indicates that this command queue should no longer
+ // be used. An example of this is if a command queue is used as a context
+ // manager, after the 'with' block exits.
+ //
+ // This mechanism is not foolproof, as it is perfectly possible to create
+ // other Python proxy objects referring to the same underlying
+ // cl_command_queue. Even so, this ought to flag a class of potentially
+ // very damaging synchronization bugs.
+ bool m_finalized;
public:
command_queue(cl_command_queue q, bool retain)
- : m_queue(q)
+ : m_queue(q), m_finalized(false)
{
if (retain)
PYOPENCL_CALL_GUARDED(clRetainCommandQueue, (q));
}
command_queue(command_queue const &src)
- : m_queue(src.m_queue)
+ : m_queue(src.m_queue), m_finalized(false)
{
PYOPENCL_CALL_GUARDED(clRetainCommandQueue, (m_queue));
}
@@ -1397,6 +1406,7 @@ namespace pyopencl
const context &ctx,
const device *py_dev=nullptr,
py::object py_props=py::none())
+ : m_finalized(false)
{
cl_device_id dev;
if (py_dev)
@@ -1513,7 +1523,24 @@ namespace pyopencl
}
const cl_command_queue data() const
- { return m_queue; }
+ {
+ if (m_finalized)
+ {
+ auto mod_warnings(py::module_::import("warnings"));
+ auto mod_cl(py::module_::import("pyopencl"));
+ mod_warnings.attr("warn")(
+ "Command queue used after exit of context manager. "
+ "This is deprecated and will stop working in 2023.",
+ mod_cl.attr("CommandQueueUsedAfterExit")
+ );
+ }
+ return m_queue;
+ }
+
+ void finalize()
+ {
+ m_finalized = true;
+ }
PYOPENCL_EQUALITY_TESTS(command_queue);
@@ -1547,7 +1574,7 @@ namespace pyopencl
case CL_QUEUE_PROPERTIES_ARRAY:
{
std::vector<cl_queue_properties> result;
- PYOPENCL_GET_VEC_INFO(CommandQueue, m_queue, param_name, result);
+ PYOPENCL_GET_VEC_INFO(CommandQueue, data(), param_name, result);
PYOPENCL_RETURN_VECTOR(cl_queue_properties, result);
}
#endif
@@ -1561,7 +1588,7 @@ namespace pyopencl
{
cl_context param_value;
PYOPENCL_CALL_GUARDED(clGetCommandQueueInfo,
- (m_queue, CL_QUEUE_CONTEXT, sizeof(param_value), ¶m_value, 0));
+ (data(), CL_QUEUE_CONTEXT, sizeof(param_value), ¶m_value, 0));
return std::unique_ptr<context>(
new context(param_value, /*retain*/ true));
}
@@ -1573,15 +1600,19 @@ namespace pyopencl
{
cl_command_queue_properties old_prop;
PYOPENCL_CALL_GUARDED(clSetCommandQueueProperty,
- (m_queue, prop, PYOPENCL_CAST_BOOL(enable), &old_prop));
+ (data(), prop, PYOPENCL_CAST_BOOL(enable), &old_prop));
return old_prop;
}
#endif
void flush()
- { PYOPENCL_CALL_GUARDED(clFlush, (m_queue)); }
+ { PYOPENCL_CALL_GUARDED(clFlush, (data())); }
void finish()
- { PYOPENCL_CALL_GUARDED_THREADED(clFinish, (m_queue)); }
+ {
+ cl_command_queue queue = data();
+
+ PYOPENCL_CALL_GUARDED_THREADED(clFinish, (queue));
+ }
// not exposed to python
int get_hex_device_version() const
@@ -1589,7 +1620,7 @@ namespace pyopencl
cl_device_id dev;
PYOPENCL_CALL_GUARDED(clGetCommandQueueInfo,
- (m_queue, CL_QUEUE_DEVICE, sizeof(dev), &dev, nullptr));
+ (data(), CL_QUEUE_DEVICE, sizeof(dev), &dev, nullptr));
std::string dev_version;
{
@@ -2318,10 +2349,12 @@ namespace pyopencl
buf = ward->m_buf.buf;
len = ward->m_buf.len;
+ cl_command_queue queue = cq.data();
+
cl_event evt;
PYOPENCL_RETRY_IF_MEM_ERROR(
PYOPENCL_CALL_GUARDED_THREADED(clEnqueueReadBuffer, (
- cq.data(),
+ queue,
mem.data(),
PYOPENCL_CAST_BOOL(is_blocking),
device_offset, len, buf,
@@ -2355,10 +2388,12 @@ namespace pyopencl
buf = ward->m_buf.buf;
len = ward->m_buf.len;
+ cl_command_queue queue = cq.data();
+
cl_event evt;
PYOPENCL_RETRY_IF_MEM_ERROR(
PYOPENCL_CALL_GUARDED_THREADED(clEnqueueWriteBuffer, (
- cq.data(),
+ queue,
mem.data(),
PYOPENCL_CAST_BOOL(is_blocking),
device_offset, len, buf,
@@ -2442,10 +2477,12 @@ namespace pyopencl
buf = ward->m_buf.buf;
+ cl_command_queue queue = cq.data();
+
cl_event evt;
PYOPENCL_RETRY_IF_MEM_ERROR(
PYOPENCL_CALL_GUARDED_THREADED(clEnqueueReadBufferRect, (
- cq.data(),
+ queue,
mem.data(),
PYOPENCL_CAST_BOOL(is_blocking),
buffer_origin, host_origin, region,
@@ -2490,10 +2527,12 @@ namespace pyopencl
buf = ward->m_buf.buf;
+ cl_command_queue queue = cq.data();
+
cl_event evt;
PYOPENCL_RETRY_IF_MEM_ERROR(
PYOPENCL_CALL_GUARDED_THREADED(clEnqueueWriteBufferRect, (
- cq.data(),
+ queue,
mem.data(),
PYOPENCL_CAST_BOOL(is_blocking),
buffer_origin, host_origin, region,
diff --git a/src/wrap_cl_part_1.cpp b/src/wrap_cl_part_1.cpp
index 4b0ec771..8d62ef5d 100644
--- a/src/wrap_cl_part_1.cpp
+++ b/src/wrap_cl_part_1.cpp
@@ -120,6 +120,7 @@ void pyopencl_expose_part_1(py::module &m)
py::arg("context"),
py::arg("device").none(true)=py::none(),
py::arg("properties")=py::cast(0))
+ .def("_finalize", &cls::finalize)
.DEF_SIMPLE_METHOD(get_info)
#if PYOPENCL_CL_VERSION < 0x1010
.DEF_SIMPLE_METHOD(set_property)
diff --git a/test/test_wrapper.py b/test/test_wrapper.py
index 9ec065f7..ff4ee0cd 100644
--- a/test/test_wrapper.py
+++ b/test/test_wrapper.py
@@ -1247,6 +1247,15 @@ def test_empty_ndrange(ctx_factory, empty_shape):
prg.add_two(queue, a.shape, None, a.data, allow_empty_ndrange=True)
+def test_command_queue_context_manager(ctx_factory):
+ ctx = ctx_factory()
+ with cl.CommandQueue(ctx) as queue:
+ q = queue
+
+ with pytest.warns(cl.CommandQueueUsedAfterExit):
+ q.finish()
+
+
if __name__ == "__main__":
# make sure that import failures get reported, instead of skipping the tests.
import pyopencl # noqa
--
GitLab