BaseContainer::GetCustomDataTypeWritableObsolete()... any side effects?
-
Hello,
I'm trying to track down a random crash in my code, that only occurs in C4D 2024. While I'm manipulating a
CustomGui
in myObjectData
's Description (e.g. by dragging a slider in thatCustomGui
), theObjectData
'sGetVirtualObjects()
is called, of course. In that function, I get theCustomGUI
's data. Sometimes, that data is thrashed. Only happens during ongoing GUI interaction.
While investigating, I found this API code in c4d_basecontainer.h:template <typename DATATYPE> DATATYPE* GetCustomDataTypeWritableObsolete(Int32 id) { // TODO: (Tilo) needs to be rewritten for COW support because this modifies data inside the container data const GeData& dat = GetData(id); return MAXON_REMOVE_CONST(dat).GetCustomDataTypeWritable<DATATYPE>(); }
This seems to be the only way to get a writable pointer to a
CustomDataType
in aBaseContainer
. Is it? Because, that comment sounds portentous.Thanks in advance!
Cheers,
Frank -
Any news on this?
-
Hi sorry I was a very packed last days.
Regarding your issue, this is correct this is the only way to get a Writable pointer to a CustomDataType in a BaseContainer. I know in some case it is not as easy as it is said, but GetVirtualObject should really just "read" parameter and output a geometry. Modifying the basecontainer is something that should be avoided since GetVirtualObject is called from different thread and therefor it is not too surprising that it crash.
The correct way would be to override the Execute method and do your change or even do your change from the main thread with ExecuteOnMainThread.
Cheers,
Maxime. -
I am not modifying any data from within
GetVirtualObjects()
, of course not.But Tilo's comment in the code sounds like the data is modified anyway, just by calling
GetCustomDataTypeWritableObsolete()
. "needs to be rewritten for COW support because this modifies data inside the container data" sounds pretty clear, that's why I'm asking.Cheers,
Frank -
Hey @fwilleke80,
I am still on vacation (and the thread is therefore still owned by Maxime), I just thought I would point out some stuff:
- When I documented the 2024 API changes I also stumbled upon the name of this function. I then asked Tilo and he then told me what now can be found in his TODO comment. The function is obsolete because it should treat custom data as COW but it does not, it uses one shared instance.
- The function is more or less perfectly fine, it 'only' violates the const-correctness of its underlying data by not treating them as immutable all the time. There is no 'data modified anyway' this function just removes the constness of the internal data so that a user could modify it. This would be better solved with COW, hence the name and comment.
I am also not quite sure about the exact nature of your problem. You use both the words crash and 'trash' (was that a typo?) but I assume you are crashing with an access violation on possibly deleted data. And you seem to crash from the main-thread? Or are you off main-thread? That is all a bit cloudy. You should post your concrete code and stack trace, otherwise it is as always hard to help you.
- When you only want to read data, you must use the read only variant BaseContrainer::GetCustomDataType of the method you are using. Fixing this will very likely not fix your crash but sticking to const-correctness is important nonetheless.
- When you are still running into access violations from a non-main thread method, you could try to overwrite
NodeData::GetAccessedObjects
just to see if it makes any difference if you manually claim a read-dependency relation to that parameter. But that should not be necessary to be done because Cinema should mark everything as a dependency when you do not implement the method manually. If overwriting the method would fix your problem, you would have found a bug which would be critical to be reported and fixed. See Node Data Access for details on the subject.
While I would not categorically rule out that there is a bug in custom data type access in
NodeData
instances, it seems quite unlikely that this is the case, given how ubiquitous custom data types are in our API. You should share your code because otherwise we cannot help you.Cheers,
Ferdinand -
Hi Ferdinand,
happy new year! Thanks for chiming in! I have fixed the crash in the meantime (don't ask how, it was all quite confusing, but it might have had to do with how I got the BaseContainer in the first place.So, if I understand this correctly, most of the structures in C4D 2024 that use COW will create a new instance when getting a writable reference or pointer, while getting a const ref or pointer will use a shared instance? And BaseContainer::GetCustomDataTypeWritableObsolete() will not do that, but still allow me to modify the pointed data? I can live with that
Posting code is difficult, as everything is part of a pretty large project. I'm sorry my questions are sometimes a bit shadowy.
Cheers,
Frank -
@fwilleke80 said in BaseContainer::GetCustomDataTypeWritableObsolete()... any side effects?:
So, if I understand this correctly, most of the structures in C4D 2024 that use COW will create a new instance when getting a writable reference or pointer, while getting a const ref or pointer will use a shared instance? And BaseContainer::GetCustomDataTypeWritableObsolete() will not do that, but still allow me to modify the pointed data? I can live with that
Exactly !
Cheers,
Maxime. -
Hey @fwilleke80,
a happy new year to you too, I was even in Berlin at New Years eve
Regarding COW, I would recommend having a look at this code example in our docs, as I tried to untangle the exact subject there.
- When you are accessing raw data such as an array of
Vector*
where there are two methods:Vector* GetVectors()
andconst Vector* GetVectors () const
, you can be fairly sure that callingVector* GetVectors()
will trigger a copy on write event. Cinema 4D has no means to detect what you are doing with this writeable array; if you only read it or if you also write it. Therefore, all data is copied as soon as you attempt to access the writeable data. We should be very careful with requesting write access in such cases because it can be a VERY expensive operation. - This is opposed by (custom) data types such as
BaseSelect
. There is aconst BaseSelect* PolygonObject::GetPolygonS const
and aBaseSelect* PolygonObject::GetWritablePolygonS
. But since the data is wrapped here by this special data type, requesting access to the writeableBaseSelect
alone will not trigger COW. It is only when you then call one of the non-const methods of this instance that its underlying data is copied.
So, in general, (custom) data types are an indicator that accessing the writeable data is not wildly expensive (because the copying can be postponed to the call of a non-const method), while raw data such as arrays (e.g., all point data) will be copied as soon as one requests write access. There are some exceptions to this, such as that copying will only occur when there is another reference holder/user to that data.
But when we are dealing with abstract/templated custom data types such as with
GetCustomDataTypeWritableObsolete
, we do not know in advance how the read/write behavior of the returned type is. So, one would have to draw a copy before returning a non-const pointer to be sure. And, yes,:: GetCustomDataTypeWritableObsolete
is currently not doing that. It simply removes the constness of the underlying data which can lead to crashes when someone or something is trying to treat such data as if it would be thread-safe/COW.Cheers,
Ferdinand - When you are accessing raw data such as an array of
-
Hi Ferdinand, thank you for the clarification!