OSL Memory Corruption (Use after free) #47008

New Issue

Ray molenkamp · 2015-12-18T01:44:39+01:00

Ray molenkamp commented

2015-12-18 01:44:39 +01:00

System Information
Windows x64

Blender Version
Broken: Latest from git
Worked: (optional)

Short description of error
Tricky one but I'll try to give a short tour.

in Session::~Session() we have this piece of code

	delete buffers;
	delete display;
	delete scene;
	delete device;

	TaskScheduler::exit();

Now here's the setup:

Scene holds the ShaderManager in m_shader manager
The PerThreadInfo class has a ShadingContext
The ShadingContext has a referene to the shader manager.
we delete the scene before we end the threads.
ShadingContext::~ShadingContext () does m_shadingsys.m_stat_contexts -= 1;
m_shadingsys doesn't exist anymore at this point in time and we're touching memory we shouldn't

Exact steps for others to reproduce the error
[With visual studio]

run a debug build
Run a scene with OSL enabled.
You should get an warning on the console about memory used after free

Tracking it down is a little harder, you have to enable the PageHeap for the process using gflags, once enabled, setup vs.net to catch exception c0000005 (access violation) (something is eating is so it doesn't break by default) and you should hit the exception in blender-app.exe!OSL::pvt::PeakCounter::operator+=(int sz)

suggested fix:

	delete buffers;
	delete display;
	TaskScheduler::exit();
	delete scene;
	delete device;

But given i'm unfamiliar with this piece of code i'm unsure if this will create other problems. (doesn't seem like it, but a sober second look never hurts)

**System Information** Windows x64 **Blender Version** Broken: Latest from git Worked: (optional) **Short description of error** Tricky one but I'll try to give a short tour. in Session::~Session() we have this piece of code ``` delete buffers; delete display; delete scene; delete device; TaskScheduler::exit(); ``` Now here's the setup: 1) Scene holds the ShaderManager in m_shader manager 2) The PerThreadInfo class has a ShadingContext 3) The ShadingContext has a referene to the shader manager. 4) we delete the scene before we end the threads. 5) ShadingContext::~ShadingContext () does m_shadingsys.m_stat_contexts -= 1; 6) m_shadingsys doesn't exist anymore at this point in time and we're touching memory we shouldn't **Exact steps for others to reproduce the error** [With visual studio] 1) run a debug build 2) Run a scene with OSL enabled. 3) You should get an warning on the console about memory used after free Tracking it down is a little harder, you have to enable the PageHeap for the process using gflags, once enabled, setup vs.net to catch exception c0000005 (access violation) (something is eating is so it doesn't break by default) and you should hit the exception in blender-app.exe!OSL::pvt::PeakCounter<int>::operator+=(int sz) suggested fix: ``` delete buffers; delete display; TaskScheduler::exit(); delete scene; delete device; ``` But given i'm unfamiliar with this piece of code i'm unsure if this will create other problems. (doesn't seem like it, but a sober second look never hurts)

Ray molenkamp commented

2015-12-18 01:44:39 +01:00

Changed status to: 'Open'

Ray molenkamp commented

2015-12-18 01:44:39 +01:00

Added subscriber: @LazyDodo

Sergey Sharybin commented

2015-12-18 12:29:38 +01:00

Added subscriber: @Sergey

Sergey Sharybin commented

2015-12-18 12:29:38 +01:00

Session is expected to be destroyed only when all the rendering is done, it should not really be destroyed prior to that. This could be just missing de-initialization happening somewhere else in the code, but it's hard to tell without being able to reproduce the issue (don't see anything running blender with )

What are the exact steps reproducing the issue? Does it happen with any scene?

Session is expected to be destroyed only when all the rendering is done, it should not really be destroyed prior to that. This could be just missing de-initialization happening somewhere else in the code, but it's hard to tell without being able to reproduce the issue (don't see anything running blender with ) What are the exact steps reproducing the issue? Does it happen with any scene?

Ray molenkamp commented

2015-12-18 13:56:23 +01:00

Session is expected to be destroyed only when all the rendering is done, it should not really be destroyed prior to that.

no problem there, the session is destroyed at an appropriate moment in time, when all rendering is completed.

This could be just missing de-initialization happening somewhere else in the code, but it's hard to tell without being able to reproduce the issue (don't see anything running blender with )

The repro steps i posted should be detailed enough to reproduce the issue on windows. Step 2 should have been 2) Render any scene with osl enabled . I noticed my mistake 2 minutes seconds after i posted but it wouldn't allow me to change it.

so more detailed recap for msvc

Make sure you run a debug build build with msvc (this might not be needed, but i noticed the problem in debug mode, so i stuck with it)
run gflags (part of the sdk, hangs out in C:\Program Files (x86)\Windows Kits\8.0\Debuggers\x64\ for me) with the following parameters in the output directory

 gflags /p /enable blender-app.exe /full

In Visual Studio go into Debug->Exceptions->Win32Exceptions and make sure to break on c0000005 when thrown
hit F5 to run.
In the render panel select cpu and enable OSL and hit render to render the default cube
Wait for rendering to finish
Boom!

If you're not on windows it'll have to go a little bit more on faith but i think this should prove the problem exists:

place breakpoints on the destructors of ShaderManager and ShadingContext,
notice that the manager gets destroyed before the context and that the context still tries to use it in it's destructor.

>Session is expected to be destroyed only when all the rendering is done, it should not really be destroyed prior to that. no problem there, the session is destroyed at an appropriate moment in time, when all rendering is completed. >This could be just missing de-initialization happening somewhere else in the code, but it's hard to tell without being able to reproduce the issue (don't see anything running blender with ) The repro steps i posted should be detailed enough to reproduce the issue on windows. Step 2 should have been 2) *Render* any scene with osl enabled . I noticed my mistake 2 minutes seconds after i posted but it wouldn't allow me to change it. so more detailed recap for msvc 1) Make sure you run a debug build build with msvc (this might not be needed, but i noticed the problem in debug mode, so i stuck with it) 2) run gflags (part of the sdk, hangs out in C:\Program Files (x86)\Windows Kits\8.0\Debuggers\x64\ for me) with the following parameters in the output directory ``` gflags /p /enable blender-app.exe /full ``` 3) In Visual Studio go into Debug->Exceptions->Win32Exceptions and make sure to break on c0000005 when thrown 4) hit F5 to run. 5) In the render panel select cpu and enable OSL and hit render to render the default cube 6) Wait for rendering to finish 7) Boom! If you're not on windows it'll have to go a little bit more on faith but i think this should prove the problem exists: 1) place breakpoints on the destructors of ShaderManager and ShadingContext, 2) notice that the manager gets destroyed before the context and that the context still tries to use it in it's destructor.

Sergey Sharybin self-assigned this 2015-12-19 15:00:36 +01:00

Sergey Sharybin commented

2015-12-19 15:00:36 +01:00

Ok, see what's happening now. Even tho we're doing all proper ownership management OSL is being lazy and uses some per-thread variables. This is happening, for example, when performing shader group optimization.

Those variables will only be freed when thread is actually exiting and there seems to be no way to force clear the storage neither from the per-thread-variable wrapper (meaning, ShadingSystemImpl can't ensure there's no handling per-thread-variables from it's destructor) and there's also no way to force such storage to clear without actually destroying the thread.

Now, your patch will work around the problem in the case when we've got single render at a time, but it'll totally fail when doing several viewport renderings or when mixing viewport and final render.

This needs some further investigation, maybe even extending OSL/boost API?

Ok, see what's happening now. Even tho we're doing all proper ownership management OSL is being lazy and uses some per-thread variables. This is happening, for example, when performing shader group optimization. Those variables will only be freed when thread is actually exiting and there seems to be no way to force clear the storage neither from the per-thread-variable wrapper (meaning, ShadingSystemImpl can't ensure there's no handling per-thread-variables from it's destructor) and there's also no way to force such storage to clear without actually destroying the thread. Now, your patch will work around the problem in the case when we've got single render at a time, but it'll totally fail when doing several viewport renderings or when mixing viewport and final render. This needs some further investigation, maybe even extending OSL/boost API?

Sergey Sharybin commented

2015-12-19 19:21:18 +01:00

One idea which might work (and in fact it seems to work on Linux, judging from debug prints and clang's asan there's no heisenbugs here) is to perform jit optimization ahead of a time using ShadingSystem->optimize_all_groups() with couple of therads used. This will make it so OSL creates threads pool and optimzation will use it's TLS. Once optimization is done, all TLS used by optimization will be freed. After that render threads will not create ShadingContext in TLS anymore.

While it's working as expected on Linux, it's still failing on Windows. Could be so OSL is still storing something in TLS for the case of Windows platform. What's also weird, disablign greedyjit does not solve memory access issues on WIndows, so it's probably something else is happening with threads.

One idea which might work (and in fact it seems to work on Linux, judging from debug prints and clang's asan there's no heisenbugs here) is to perform jit optimization ahead of a time using `ShadingSystem->optimize_all_groups()` with couple of therads used. This will make it so OSL creates threads pool and optimzation will use it's TLS. Once optimization is done, all TLS used by optimization will be freed. After that render threads will not create ShadingContext in TLS anymore. While it's working as expected on Linux, it's still failing on Windows. Could be so OSL is still storing something in TLS for the case of Windows platform. What's also weird, disablign `greedyjit` does not solve memory access issues on WIndows, so it's probably something else is happening with threads.

Sergey Sharybin commented

2015-12-20 09:38:16 +01:00

Ok, had a mistake in original greedyjut change i was talking about yesterday. This patch should solve the bad memory access:

P304: Fix #47008

diff --git a/intern/cycles/render/osl.cpp b/intern/cycles/render/osl.cpp
index 4933ed6..e9b51ab 100644
--- a/intern/cycles/render/osl.cpp
+++ b/intern/cycles/render/osl.cpp
@@ -125,11 +125,21 @@ void OSLShaderManager::device_update(Device *device, DeviceScene *dscene, Scene
 
 	device_update_common(device, dscene, scene, progress);
 
-	/* greedyjit test
 	{
+		/* Perform greedyjit optimization.
+		 *
+		 * This might waste time on optimizing gorups which are never actually
+		 * used, but this prevents OSL from allocating data on TLS at render
+		 * time.
+		 *
+		 * This is much better for us because this way we aren't required to
+		 * stop task scheduler threads to make sure all TLS is clean and don't
+		 * have issues with TLS data free accessing freed memory if task scheduler
+		 * is being freed after the Session is freed.
+		 */
 		thread_scoped_lock lock(ss_shared_mutex);
-		ss->optimize_all_groups();
-	}*/
+		ss->optimize_all_groups(4);
+	}
 }
 
 void OSLShaderManager::device_free(Device *device, DeviceScene *dscene, Scene *scene)
@@ -195,7 +205,7 @@ void OSLShaderManager::shading_system_init()
 		ss_shared->attribute("lockgeom", 1);
 		ss_shared->attribute("commonspace", "world");
 		ss_shared->attribute("searchpath:shader", path_get("shader"));
-		//ss_shared->attribute("greedyjit", 1);
+		ss_shared->attribute("greedyjit", 1);
 
 		VLOG(1) << "Using shader search path: " << path_get("shader");

It's not totally optimal from the performance point of view, but unless i'm wrong it'll only be some constant delay before the rendering actually starts.

Ok, had a mistake in original greedyjut change i was talking about yesterday. This patch should solve the bad memory access: [P304: Fix #47008](https://archive.blender.org/developer/P304.txt) ``` diff --git a/intern/cycles/render/osl.cpp b/intern/cycles/render/osl.cpp index 4933ed6..e9b51ab 100644 --- a/intern/cycles/render/osl.cpp +++ b/intern/cycles/render/osl.cpp @@ -125,11 +125,21 @@ void OSLShaderManager::device_update(Device *device, DeviceScene *dscene, Scene device_update_common(device, dscene, scene, progress); - /* greedyjit test { + /* Perform greedyjit optimization. + * + * This might waste time on optimizing gorups which are never actually + * used, but this prevents OSL from allocating data on TLS at render + * time. + * + * This is much better for us because this way we aren't required to + * stop task scheduler threads to make sure all TLS is clean and don't + * have issues with TLS data free accessing freed memory if task scheduler + * is being freed after the Session is freed. + */ thread_scoped_lock lock(ss_shared_mutex); - ss->optimize_all_groups(); - }*/ + ss->optimize_all_groups(4); + } } void OSLShaderManager::device_free(Device *device, DeviceScene *dscene, Scene *scene) @@ -195,7 +205,7 @@ void OSLShaderManager::shading_system_init() ss_shared->attribute("lockgeom", 1); ss_shared->attribute("commonspace", "world"); ss_shared->attribute("searchpath:shader", path_get("shader")); - //ss_shared->attribute("greedyjit", 1); + ss_shared->attribute("greedyjit", 1); VLOG(1) << "Using shader search path: " << path_get("shader"); ``` It's not totally optimal from the performance point of view, but unless i'm wrong it'll only be some constant delay before the rendering actually starts.

blender-admin commented

2016-01-03 14:28:33 +01:00

This issue was referenced by a3df65dea8

This issue was referenced by a3df65dea819309496447a26ddf4d7dbe0c3203a

Sergey Sharybin commented

2016-01-03 14:37:16 +01:00

Changed status from 'Open' to: 'Resolved'

Sergey Sharybin closed this issue

2016-01-03 14:37:16 +01:00

blender-admin commented

2016-01-24 12:48:49 +01:00

This issue was referenced by blender/cycles@626e479945

This issue was referenced by blender/cycles@626e4799454c72cf55980b48a9b719d994b1e154

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

OSL Memory Corruption (Use after free) #47008