Fix RNA_property_pointer_get creating data in non-thread-safe way. #90170

New Issue

Bastien Montagne · 2021-07-26T15:47:14+02:00

Bastien Montagne commented

2021-07-26 15:47:14 +02:00

Current RNA_property_pointer_get code creates the data if missing in IDProperty case (see rna_acces.c). With a comment that is is very bad to create data in accessor code...

This comes from initial twelve years old commit (4fb19159ea) and was never addressed since then.

This is currently causing memleaks in liboverride diffing code, which is parallelized on the ID level. The problem shows up when you have several overrides of the same linked ID, then diffing code can access concurrently to the same linked ID and generate IDProperty several time for the same entry.

I can think of several ways to solve this issue:

Add a lock (mutex, spinlock...) protection that specific bad piece of code.
Never generate IDproperty data for linked IDs (since it won't be saved in any case).
Try to actually fix that bad code - not sure what would be the implications of not silently generating those IDProperty data on access though... Have the feeling this could be quite API-disruptive?

Current `RNA_property_pointer_get` code creates the data if missing in IDProperty case (see `rna_acces.c`). With a comment that is is very bad to create data in accessor code... This comes from initial twelve years old commit (4fb19159ea) and was never addressed since then. This is currently causing memleaks in liboverride diffing code, which is parallelized on the ID level. The problem shows up when you have several overrides of the same linked ID, then diffing code can access concurrently to the same linked ID and generate IDProperty several time for the same entry. ------------------------------ I can think of several ways to solve this issue: - Add a lock (mutex, spinlock...) protection that specific bad piece of code. - Never generate IDproperty data for linked IDs (since it won't be saved in any case). - Try to actually fix that bad code - not sure what would be the implications of not silently generating those IDProperty data on access though... Have the feeling this could be quite API-disruptive?

Bastien Montagne commented

2021-07-26 15:47:14 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Bastien Montagne commented

2021-07-26 15:47:14 +02:00

Added subscribers: @mont29, @brecht

Brecht Van Lommel commented

2021-07-26 15:57:14 +02:00

I suggest to just add a mutex lock. It's difficult to work around at this point.

The issue is that if you have some nested properties, like scene.cycles.samples, it would be nice if we could only allocate the cycles ID property group when one if its properties is set for the first time. But I can't think of a practical way to create an RNA pointer to scene.cycles when it does not exist yet, without adding a lot of complexity.

For the library diffing code specifically, it could perhaps not enter into that struct when it has not been allocated, but doesn't solve the potential issue in other cases.

I suggest to just add a mutex lock. It's difficult to work around at this point. The issue is that if you have some nested properties, like `scene.cycles.samples`, it would be nice if we could only allocate the `cycles` ID property group when one if its properties is set for the first time. But I can't think of a practical way to create an RNA pointer to `scene.cycles` when it does not exist yet, without adding a lot of complexity. For the library diffing code specifically, it could perhaps not enter into that struct when it has not been allocated, but doesn't solve the potential issue in other cases.

blender-admin commented

2021-07-28 17:36:40 +02:00

This issue was referenced by 317f09ebf9

This issue was referenced by 317f09ebf990ab4a5d033bab61a0aa8816772615

Bastien Montagne commented

2021-08-05 12:15:41 +02:00

Changed status from 'Confirmed' to: 'Resolved'

Bastien Montagne closed this issue

2021-08-05 12:15:41 +02:00

Bastien Montagne self-assigned this 2021-08-05 12:15:41 +02:00

Thomas Dinges added this to the 3.0 milestone 2023-02-08 15:59:05 +01:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix RNA_property_pointer_get creating data in non-thread-safe way. #90170

Fix `RNA_property_pointer_get` creating data in non-thread-safe way. #90170