Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

App crashes when using Sensor.Start(FrameCallback) #1250

Closed
baSSiLL opened this issue Feb 28, 2018 · 5 comments
Closed

App crashes when using Sensor.Start(FrameCallback) #1250

baSSiLL opened this issue Feb 28, 2018 · 5 comments

Comments

@baSSiLL
Copy link
Contributor

baSSiLL commented Feb 28, 2018

Hi
I'm using D415 on PC with .NET wrapper.
I prefer push model for consuming frames. The only API I've found for that is Sensor.Start method taking a callback delegate. However, after calling it the app crashes suddenly (after processing a few frames) and debug output contains message about invoking a garbage-collected delegate.

Digging into the code revealed that Sensor.Start actually creates another ad-hoc delegate which it then passes to unmanaged code. But because this ad-hoc delegate is not referenced by any managed object it becomes a subject for garbage collection after leaving the method body. So, after it is collected, calls from unmanaged code lead to error.

To mitigate this issue, a reference to delegate should be stored somewhere. Some options include:

  • A private field in Sensor class. Storing a single delegate instance should be sufficient because no more than a single callback can be activated simultaneously. Care should be taken here regarding thread-safety.

  • Make the Start method return some token (a "black-box" data structure) which holds a reference to the delegate. And require to pass this token in a subsequent invocation of Stop method. This should force a responsible user of the class to hold the token (effectively keeping the delegate alive) for a period when the callback can be actually called.

For the time being I've worked the issue around by making a replacement method for Sensor.Start in my code which calls unmanaged rs2_start and does proper work of storing a reference to a delegate passed.

@dorodnic
Copy link
Contributor

Hi @baSSiLL
Thanks for reporting this issue, we will try to reproduce and push a work-around as soon a possible.
You are also welcomed to contribute to this project if you like.

@dorodnic
Copy link
Contributor

dorodnic commented May 6, 2018

Hi @baSSiLL
Sorry for long delay. This should be resolved in #1650, and released as part of 2.11.0

@baSSiLL
Copy link
Contributor Author

baSSiLL commented May 7, 2018

Thanks @dorodnic
Looks like the original issue is fixed. BTW it's not mentioned in release notes from what I read.

However, this API still appears of not much use to me because I need an instance of VideoFrame type to be used later in processing blocks. And AFAIK there is no way currently to convert from Frame to VideoFrame or any other specific frame type.
Probable solutions would be:

  1. Make Start method generic. Looks the most convenient. However its implementation would most likely resort to a switch by specific frame types which is bad from the point of maintenance.
void Start(Action<TFrame> callback) where TFrame : Frame;
  1. Automatically create an instance of the most specific frame type suitable for an actual frame so a client could do a cast. I did not check though whether it is feasible to determine such type.

  2. Add copy constructors accepting Frame to each of the specific frame types. So one can easily construct instance of a needed type having instance of other type. Optionally may add explicit cast operators to specific types from Frame. Has flaw of making possible to convert between incompatible frame types.

  3. Make m_instance field public same as in Sensor class. More simple for implementation and less safe variation of previous one.

I'd vote for the 3d option. Let me know of your thoughts on this.

@RealSense-Customer-Engineering
Copy link
Collaborator

[Realsense Customer Engineering Team Comment]
hi @baSSiLL,

would like to know the new tool "convert" can fit your need?
thanks.

@baSSiLL
Copy link
Contributor Author

baSSiLL commented Feb 8, 2019

Sorry for a late reply :)
Do not actually get what is that "convert" tool. Anyway, I see that there is a new method Frame.CreateFrame which is used widely to create an instance of appropriate specific subclass of Frame. That is in line with my proposal #2 from the latest comment. However, it's not used in the places where I suggested - when providing a frame for FrameCallback delegates (in Sensor.Start and Pipeline.Start methods). In those places a frame object is still created using Frame constructor.
Thankfully I can workaround this in my callback by invoking Frame.Clone method (which calls CreateFrame under the hood). But would be great if a frame passed to the callback was already constructed using CreateFrame and had a specific type (e.g. DepthFrame or VideoFrame).

gwen2018 pushed a commit to gwen2018/librealsense that referenced this issue Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants