pybind11 icon indicating copy to clipboard operation
pybind11 copied to clipboard

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

Open 2bam opened this issue 8 years ago • 1 comments

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

2bam avatar Oct 26 '17 19:10 2bam