jherico / OculusSDK

Oculus SDK for Virtual Reality
Other
202 stars 114 forks source link

SDK will crash if you instantiate a SensorFusion or SensorFilter object on the stack before OVR::System::Init() #25

Closed jherico closed 10 years ago

jherico commented 11 years ago

The new implementation of SensorFilter instantiates a CircularBuffer object as a member, which in turn uses teh OVR::Allocator in it's constructor. This means that if you have a SensorFilter instance (or a SensorFusion instance, which contains a SensorFilter) in your class and the class is instantiated before OVR::System::Init() is called, then it will crash with a null pointer access when it attempts to use the allocator.

jherico commented 10 years ago

Closing this out as the C API makes this a non-issue.

jherico commented 10 years ago

It's a real issue with the C++ API, but not for the C API. In the C API there's no equivalent for the SensorFusion object. Instead you use ovrHmd_GetSensorState() or (more likely) use the ovrPosef value returned by ovrHmd_BeginEyeRender() to fetch the orientation and position of the headset.

You still have to call ovr_Initialize before you use any of those functions, but there's no opportunity for you to declare an object which would require Oculus SDK functions that haven't yet been enabled.

ChristophHaag commented 10 years ago

Sorry, deleted my comment because I thought it may just have been noise.

It was just confusing to see this closed for being a non-issue because of the C API because there is still the C++ API and other programs do use it..

I mean a workaround would have been nice, where the initialization of OVR::SensorFusion is maybe moved into System::Init() (or something like that, I'm not actually familiar with the code)

jherico commented 10 years ago

The impression I get is that Oculus is going to treat the C API as their publicly facing API going forward and treat the C++ API as more of an internal implementation detail. I don't know of any use cases that would require the user to access the SensorFusion class directly.

If you don't want to update to the C API right now, the simple solution is to not allocate any SensorFusion objects before calling the OVR init functions. This means not making them globals nor members of classes that would be instantiated before the OVR SDK was initialized. An easy way to do this is to only use SensorFusion* or std::shared_ptr instead of just SensorFusion.

ChristophHaag commented 10 years ago

Well, it might not always be a "simple solution", if for whatever reason you have a big program that uses a global reference to a SensorFusion object, it will always initialize before you can call the ovr init function. In this example the changes are relatively tame: https://bitbucket.org/druidsbane/ibex/issue/11/linux-segfault#comment-10888822 (I think, at least it compiles) but for really large programs it can probably be some problem..

Anyway, let's just blame upstream and move on. :)

And thanks for the explanations.