pybind / pybind/pybind11

Weak pointer semantics for holder types, not updated in type_caster_generic::cast if internal ptr cached.

Open
#1,157 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
18k
Forks
2.3k
Avg merge
5d 17h
Merged PRs (30d)
10

Description

Issue description

I'm trying to make a holder type for versioned weak pointers (w/indexed pools), as many videogames do for the entitiy-component-system pattern. So the smart pointer is actually an index to the pool memory chunk and a version which get() resolves ok or throws an exception on version mismatch (object was destroyed, dangling holder).

The problem I see is that the "instance cache check" only checks with the internal pointer (in type_caster_generic::cast) and inc_refs discarding the "holder" parameter altogether, which has it's state changed (version number).

Is there a way to go around it? I'd like to keep the holder transparent syntax but check every time.

Notice that I marked //HACK for another minor bug workaround, it wont compile unless I have a "held-type ptr ctor" available, although it's never actually called for copyable holders but non-constructible-from-python types.

Thanks

Setup

Microsoft Visual Studio Community 2015 Version 14.0.25431.01 Update 3
pybind11 master (5c7a290d374b8b6cc3bb3d1d6f1316547ba118f2)
Python 3.4.0 32-bit release

Reproducible example code

C++ code
#define HAVE_ROUND
#include <pybind11/pybind11.h>
#include <iostream>

//////////////
// TYPES
//////////////
class C {
public:
	int something;
};

template<typename T>
struct LousyPool {
	static T poolbuf[1];	
	static int poolver[1];
};

C LousyPool<C>::poolbuf[1] {0};
int LousyPool<C>::poolver[1] {0};

//////////////
// HOLDER TYPE
//////////////

template<typename T>
class Holder {
public:
	int index;
	int version;

	Holder() : index(-1), version(-1) {}
	Holder(int index, int version) : index(index), version(version) {}
	Holder(const Holder& o) : index(o.index), version(o.version) {}

	//HACK: Otherwise it wont compile @ pybind11.h(1280)
	explicit Holder(const T* junk) { throw std::exception("Never called"); }

	T *get() const {
		std::cerr << "GET " << this << " " << index << " " << version << " " << LousyPool<T>::poolver[index] << std::endl;
		if(version != LousyPool<T>::poolver[index])
			throw std::exception(("Version mismatch, dangling ref version " + std::to_string(version) + " current one " + std::to_string(LousyPool<T>::poolver[index])).c_str());
		return LousyPool<T>::poolbuf+index;
	}
};
PYBIND11_DECLARE_HOLDER_TYPE(T, Holder<T>);

//////////////
// FUNCS
//////////////

Holder<C> c_acquire() {
	auto p = Holder<C>(0, LousyPool<C>::poolver[0]);
	p.get()->something = 123;
	return p;
}

int c_touch(Holder<C> p) {
	return p.get()->something;
}

void c_release(Holder<C> p) {
	LousyPool<C>::poolver[p.index]++;
}

//////////////
// BINDINGS
//////////////

PYBIND11_MODULE(pybind11_bugs_lib, m) {
	pybind11::class_<C, Holder<C>>(m, "C")
		.def_readwrite("something", &C::something)
		;	
	m.def("c_acquire", &c_acquire);
	m.def("c_release", &c_release);
	m.def("c_touch", &c_touch);
}

Python code
from pybind11_bugs_lib import *

x = c_acquire()
print(x.something)	# Raw access: OK... for now
print(c_touch(x)) 	# Held access: OK

c_release(x)		# Change version
y = c_acquire()		# Re-acquire (version+1)

try:
	print(x.something)	# PROBLEM: Still accessing a cached dangling pointer
	print(c_touch(x))	# Should throw exception. OK, it IS old
except Exception as e:
	print(e)

print(y.something)	# Raw access: OK... for now
print(c_touch(y))	# ERROR: Should NOT throw exception. Version was not updated because holder is cached as well

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Build and run the provided C++ and Python reproducer, then inspect type_caster_generic::cast and inc_refs in pybind11.h, including the area around line 1280 mentioned by the report. Verify that cached holder state is handled correctly: the old x access raises after c_release, while the reacquired y remains usable.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.