boostorg / boostorg/python

boost::python::objects::add_cast can make incorrect connections

Open
#103 11 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
537
Forks
223
Avg merge
11h 22m
Merged PRs (30d)
2

Description

(this was previously logged on trac: https://svn.boost.org/trac/boost/ticket/12807 before I noticed boost::python issues are meant to be reported on github)

We seem to be fairly unique in using add_cast (at least I couldn't find many other reports on Google), so maybe we're not playing by the rules. However.....

We've observed that when adding lots of casting relationships via add_cast, sometimes the relationships aren't setup properly.

I've narrowed this down to demand_types() in inheritance.cpp

It appropriately reserves space to avoid overall reallocation for the type_index vector, however as best I can tell, demand_type() will still shift elements in the vector when it calls insert() with an iterator in the middle of the vector. As a result, we're observing:

- demand_types() calls demand_type() to set 'first'
- demand_types() calls demand_type() to set 'second'
- this call to demand_type() calls insert() which shifts the element that 'first' was previously pointing at

As a result, 'first' ends up pointing at the wrong element, and the wrong pair of vertices are connected in the adjacency_list, effectively setting up a bogus casting relationship.

The eventual impact is that users' python scripts start failing with error messages about their arguments not matching the C++ signature.

Maybe we're breaking a contract by calling add_cast without somehow registering both end-points into the adjacency_list and, if so, I'd love to get some best-practice guidance.

If not, and this is a legitimate issue, here's the patch we're putting in place for the time being:

```
type_index_t::iterator first = demand_type(t1);
type_index_t::iterator second = demand_type(t2);
+ first = demand_type(t1);
```

The first two (unchanged) calls still ensure both types have associated vertices, and the last (new) call compensates for the possibility that the second call invalidated the result from the first call.

I hope this report is useful. It's my first, so please let me know if I can provide any more info! I'll try to upload a repro case later if I can extract it from our production codebase.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.