Skip to content

Commit be944da

Browse files
Copilotgreenc-FNAL
andcommitted
Fix transform node null store cache on exception and restore NUMPY_ARRAY_CONVERTER null check
Two fixes for py:vectypes: 1. declared_transform.hpp: When stores_.insert() creates a new entry with a default (null) value and the user callback throws, the null entry remained in the cache. On subsequent messages with the same hash, the null store was reused and sent downstream, causing null pointer propagation through the graph. Fix: wrap the callback in try/catch and erase the entry on exception. 2. modulewrap.cpp: Restore the null check for pyobj in NUMPY_ARRAY_CONVERTER that was removed in PR #213. Without it, PyArray_Check(nullptr) and Py_DECREF(nullptr) cause undefined behavior, leading to intermittent SEGFAULTs or incorrect behavior. Co-authored-by: greenc-FNAL <2372949+greenc-FNAL@users.noreply.github.com>
1 parent baf4717 commit be944da

File tree

2 files changed

+21
-12
lines changed

2 files changed

+21
-12
lines changed

phlex/core/declared_transform.hpp

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -98,18 +98,23 @@ namespace phlex::experimental {
9898
} else {
9999
accessor a;
100100
if (stores_.insert(a, store->index()->hash())) {
101-
auto result = call(ft, messages, std::make_index_sequence<N>{});
102-
++calls_;
103-
++product_count_[store->index()->layer_hash()];
104-
products new_products;
105-
new_products.add_all(output_, std::move(result));
106-
a->second = std::make_shared<product_store>(
107-
store->index(), this->full_name(), std::move(new_products));
108-
109-
message const new_msg{a->second, message_id};
110-
stay_in_graph.try_put(new_msg);
111-
to_output.try_put(new_msg);
112-
flag_for(store->index()->hash()).mark_as_processed();
101+
try {
102+
auto result = call(ft, messages, std::make_index_sequence<N>{});
103+
++calls_;
104+
++product_count_[store->index()->layer_hash()];
105+
products new_products;
106+
new_products.add_all(output_, std::move(result));
107+
a->second = std::make_shared<product_store>(
108+
store->index(), this->full_name(), std::move(new_products));
109+
110+
message const new_msg{a->second, message_id};
111+
stay_in_graph.try_put(new_msg);
112+
to_output.try_put(new_msg);
113+
flag_for(store->index()->hash()).mark_as_processed();
114+
} catch (...) {
115+
stores_.erase(a);
116+
throw;
117+
}
113118
} else {
114119
stay_in_graph.try_put({a->second, message_id});
115120
}

plugins/python/src/modulewrap.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,10 @@ namespace {
405405
\
406406
auto vec = std::make_shared<std::vector<cpptype>>(); \
407407
\
408+
if (!pyobj) { \
409+
return vec; \
410+
} \
411+
\
408412
/* TODO: because of unresolved ownership issues, copy the full array contents */ \
409413
if (PyArray_Check((PyObject*)pyobj)) { \
410414
PyArrayObject* arr = (PyArrayObject*)pyobj; \

0 commit comments

Comments
 (0)