Mantaflow: several crashes due to null pointers. #72894

New Issue

Ray molenkamp · 2020-01-04T10:08:26+01:00

Ray molenkamp commented

2020-01-04 10:08:26 +01:00

System Information
Operating system: Windows 64 bit
Graphics card: GTX1660

Blender Version
Broken: aad09525fb
Worked: before mantaflow merge

Short description of error
There are several null derefs in the mantaflow code, running a debug build bring them out virtually instantly

the first one is here where obstacles is dereferenced without checking it is a valid pointer, a few lines later, a similar field is checked for null before de-referencing so it's probably needed here as well, however this may be masking the actual issue that the field should not be null in the first place.

When adding the check and trying again it hits another null pointer here where shadow is a null pointer.

Exact steps for others to reproduce the error

Make a debug build
on the default cube scene use quick smoke
Bake
poof

**System Information** Operating system: Windows 64 bit Graphics card: GTX1660 **Blender Version** Broken: aad09525fb Worked: before mantaflow merge **Short description of error** There are several null derefs in the mantaflow code, running a debug build bring them out virtually instantly the first one is [here ](https://developer.blender.org/diffusion/B/browse/master/source/blender/blenkernel/intern/fluid.c$1025) where `obstacles` is dereferenced without checking it is a valid pointer, a few lines later, a similar field is checked for null before de-referencing so it's probably needed here as well, however this may be masking the actual issue that the field should not be null in the first place. When adding the check and trying again it hits another null pointer [here ](https://developer.blender.org/diffusion/B/browse/master/source/blender/blenkernel/intern/fluid.c$4025) where `shadow` is a null pointer. **Exact steps for others to reproduce the error** - Make a debug build - on the default cube scene use `quick smoke` - Bake - *poof*

Ray molenkamp commented

2020-01-04 10:08:26 +01:00

Added subscriber: @LazyDodo

Sebastián Barschkis was assigned by Ray molenkamp

2020-01-04 10:10:21 +01:00

Ray molenkamp commented

2020-01-04 10:10:21 +01:00

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

blender-admin commented

2020-01-15 15:50:02 +01:00

This issue was referenced by adcc9d014c

This issue was referenced by adcc9d014cc2ec27fea74168bdb4c47fddedbeb3

blender-admin commented

2020-01-15 15:50:02 +01:00

This issue was referenced by 2ff3877f71

This issue was referenced by 2ff3877f71fb0e8c806cdd02825ebdf7d6b8b9cc

Sebastián Barschkis commented

2020-01-15 15:52:21 +01:00

Changed status from 'Confirmed' to: 'Resolved'

Sebastián Barschkis closed this issue

2020-01-15 15:52:21 +01:00

Sebastián Barschkis commented

2020-01-15 15:57:22 +01:00

@LazyDodo Just pushed an update that gets rid of the entire loop, i.e. the functionality is now directly in Mantaflow. It's much cleaner this way.

Can you check if the issue with the shadow null pointer persists and re-open the task if it does?

@LazyDodo Just pushed an update that gets rid of the entire loop, i.e. the functionality is now directly in Mantaflow. It's much cleaner this way. Can you check if the issue with the `shadow` null pointer persists and re-open the task if it does?

Ray molenkamp commented

2020-01-15 16:50:44 +01:00

Changed status from 'Resolved' to: 'Confirmed'

Ray molenkamp reopened this issue

2020-01-15 16:50:44 +01:00

Ray molenkamp commented

2020-01-15 16:50:44 +01:00

the obstacles crash is gone, shadow however is still an issue, tested on c27acbcfb7

the obstacles crash is gone, shadow however is still an issue, tested on c27acbcfb7

Sebastián Barschkis commented

2020-01-21 20:19:39 +01:00

Was able to reproduce this. Interestingly this bug (i.e. immediate crash with Quick Smoke) only shows up on Windows Debug builds. Still investigating why that is the case. For some reason the grid pointers from Manta don't transfer into Blender.

Ray molenkamp commented

2020-01-22 00:57:39 +01:00

~~where should these pointers be transferred? happy to look at it, odd compiler bugs is kinda my jam :)~~

all looks fine until pyObjectToString where

  PyObject *encoded = PyUnicode_AsUTF8String(inputObject);
  PyObject_Print(encoded, stdout, 0);

still look good

but

  char *result = PyBytes_AsString(encoded);

returns a pointer to valid bufferr, but not containing the data expected.

~~where should these pointers be transferred? happy to look at it, odd compiler bugs is kinda my jam :)~~ all looks fine until `pyObjectToString` where ``` PyObject *encoded = PyUnicode_AsUTF8String(inputObject); PyObject_Print(encoded, stdout, 0); ``` still look good but ``` char *result = PyBytes_AsString(encoded); ``` returns a pointer to valid bufferr, but not containing the data expected.

Ray molenkamp commented

2020-01-22 06:06:39 +01:00

it's a dangling pointer:

static char *pyObjectToString(PyObject *inputObject)
{
  PyGILState_STATE gilstate = PyGILState_Ensure();

  PyObject *encoded = PyUnicode_AsUTF8String(inputObject); <---- constructs new object
  char *result = PyBytes_AsString(encoded);  <--- gets pointer to data inside this object, does NOT make a copy
  Py_DECREF(encoded); <--- deletes object, python manages the memory, doesn't instantly free it on release builds, but invalidates the buffer contents on debug
  Py_DECREF(inputObject);

  PyGILState_Release(gilstate);
  return result; <-- returns dangling pointer to caller 
}

it's a dangling pointer: ``` static char *pyObjectToString(PyObject *inputObject) { PyGILState_STATE gilstate = PyGILState_Ensure(); PyObject *encoded = PyUnicode_AsUTF8String(inputObject); <---- constructs new object char *result = PyBytes_AsString(encoded); <--- gets pointer to data inside this object, does NOT make a copy Py_DECREF(encoded); <--- deletes object, python manages the memory, doesn't instantly free it on release builds, but invalidates the buffer contents on debug Py_DECREF(inputObject); PyGILState_Release(gilstate); return result; <-- returns dangling pointer to caller } ```

blender-admin commented

2020-01-22 11:17:34 +01:00

This issue was referenced by c4b5279bbc

This issue was referenced by c4b5279bbca4db2ee88e0451fe6d41175e26719a

Sebastián Barschkis commented

2020-01-22 11:18:18 +01:00

Changed status from 'Confirmed' to: 'Resolved'

Sebastián Barschkis closed this issue

2020-01-22 11:18:18 +01:00

Sebastián Barschkis commented

2020-01-22 11:26:05 +01:00

@LazyDodo Good catch! Removing the call to Py_DECREF(encoded); should do the trick. Those objects (i.e. manta grid objects, mesh objects, etc.) should really only get deleted via the del calls directly in Python.
My Windows debug build runs fine now, but please reopen if you notice something odd.

@LazyDodo Good catch! Removing the call to `Py_DECREF(encoded);` should do the trick. Those objects (i.e. manta grid objects, mesh objects, etc.) should really only get deleted via the `del` calls directly in Python. My Windows debug build runs fine now, but please reopen if you notice something odd.

Ray molenkamp commented

2020-01-22 15:19:24 +01:00

I'm pretty sure you just swung in the opposite direction and rather than a danging pointer created a memory leak, PyUnicode_AsUTF8String creates a new object, which you then lose the reference to.

Ray molenkamp commented

2020-01-22 16:28:44 +01:00

Changed status from 'Resolved' to: 'Confirmed'

Ray molenkamp reopened this issue

2020-01-22 16:28:44 +01:00

Sebastián Barschkis commented

2020-01-22 16:48:01 +01:00

@LazyDodo Thoughts on be7571a5e4?

Ray molenkamp commented

2020-01-22 17:04:40 +01:00

Added subscriber: @ideasman42

Ray molenkamp commented

2020-01-22 17:04:40 +01:00

That'll do it! The whole chain of pointer->string->pythonobject->string->pointer gives me the heebie jeebies but at-least it appears to be done without causing any leaks/dangling pointers.

although...... that Py_DECREF(inputObject); does look suspicious, why is it there? but i fully admit, python is really not my area of expertise @ideasman42 may have better insights here.

That'll do it! The whole chain of pointer->string->pythonobject->string->pointer gives me the heebie jeebies but at-least it appears to be done without causing any leaks/dangling pointers. although...... that ` Py_DECREF(inputObject);` does look suspicious, why is it there? but i fully admit, python is really not my area of expertise @ideasman42 may have better insights here.

Sebastián Barschkis commented

2020-01-22 18:40:50 +01:00

I've added some more clarifications in the comments of ca7bd3f1c3. The Py_DECREF(inputObject); is needed because inputObject is a new reference whose responsibility was handed over.

I've added some more clarifications in the comments of ca7bd3f1c3. The `Py_DECREF(inputObject);` is needed because `inputObject` is a new reference whose responsibility was handed over.

Ray molenkamp commented

2020-01-22 21:04:16 +01:00

Changed status from 'Confirmed' to: 'Resolved'

Ray molenkamp closed this issue

2020-01-22 21:04:16 +01:00

Ray molenkamp commented

2020-01-22 21:04:16 +01:00

if you think it's safe, no need to keep this open

Sebastián Barschkis commented

2020-01-22 21:18:17 +01:00

Yes, I think it's safe. Thanks again for helping with this issue!

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

Mantaflow: several crashes due to null pointers. #72894