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.

Sign up for GitHub

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

improvements to avoids crashes #13615

Open
wants to merge 3 commits into
base: development
Choose a base branch
from
Open

improvements to avoids crashes #13615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e0c1532
improvements to avoid crashes
remibettan Dec 18, 2024
d89160e
comment added
remibettan Dec 22, 2024
bd6a0aa
comment updated, branch rebased
remibettan Dec 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/ds/d400/d400-device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,12 +487,17 @@ namespace librealsense
std::vector<std::shared_ptr<platform::uvc_device>> depth_devices;
auto depth_devs_info = filter_by_mi( all_device_infos, 0 );

if ( depth_devs_info.empty() )
for (auto&& info : depth_devs_info) // Filter just mi=0, DEPTH
{
auto depth_uvc_device = get_backend()->create_uvc_device(info);
if (depth_uvc_device)
depth_devices.push_back(depth_uvc_device);
}

if (depth_devs_info.empty() || depth_devices.empty())
{
throw backend_exception("cannot access depth sensor", RS2_EXCEPTION_TYPE_BACKEND);
}
for (auto&& info : depth_devs_info) // Filter just mi=0, DEPTH
depth_devices.push_back( get_backend()->create_uvc_device( info ) );

std::unique_ptr< frame_timestamp_reader > timestamp_reader_backup( new ds_timestamp_reader() );
frame_timestamp_reader* timestamp_reader_from_metadata;
Expand Down
12 changes: 8 additions & 4 deletions src/ds/d500/d500-device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,13 +359,17 @@ namespace librealsense
std::vector<std::shared_ptr<platform::uvc_device>> depth_devices;
auto depth_devs_info = filter_by_mi( all_device_infos, 0 );

if ( depth_devs_info.empty() )
for (auto&& info : depth_devs_info) // Filter just mi=0, DEPTH
{
throw backend_exception("cannot access depth sensor", RS2_EXCEPTION_TYPE_BACKEND);
auto depth_uvc_device = get_backend()->create_uvc_device(info);
if (depth_uvc_device)
depth_devices.push_back(depth_uvc_device);
}

for( auto & info : depth_devs_info ) // Filter just mi=0, DEPTH
depth_devices.push_back( get_backend()->create_uvc_device( info ) );
if (depth_devs_info.empty() || depth_devices.empty())
{
throw backend_exception("cannot access depth sensor", RS2_EXCEPTION_TYPE_BACKEND);
}

std::unique_ptr< frame_timestamp_reader > timestamp_reader_backup( new ds_timestamp_reader() );
std::unique_ptr<frame_timestamp_reader> timestamp_reader_metadata(new ds_timestamp_reader_from_metadata(std::move(timestamp_reader_backup)));
Expand Down
10 changes: 6 additions & 4 deletions src/win/win-helpers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,14 +491,16 @@ namespace librealsense
}

// Enumerate all imaging devices
for (int member_index = 0; ; ++member_index)
// 0x10000 ia a large enough limit to permit maximum connection, while avoiding infinite loop
for (DWORD member_index = 0; member_index < 0x10000; ++member_index)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain? add comment? why < 0x10000?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only some limit to avoid infinite loop. No real meaning for this number - only assuming that there will be no system with 0x10000 (65,536) devices.
I ll add a comment.

{
// Get device information element from the device information set
if (SetupDiEnumDeviceInfo(device_info, member_index, &devInfo) == FALSE)
{
if( GetLastError() == ERROR_NO_MORE_ITEMS )
break; // stop when none left
continue; // silently ignore other errors
DWORD last_error = GetLastError();
if ((last_error == ERROR_NO_MORE_ITEMS) || (last_error == ERROR_INVALID_HANDLE))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also explain why this was added?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The customer was complaining that the error ERROR_INVALID_HANDLE happened sometimes, and that this was leading to infinite loop (see in ticket).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this can happen when looping the devices even if not finished yet or only after.
If we will get it in the middle we will nor enumerate the device right?
On Linux it will be forever as the trigger is udev.
We need to make sure we need to break on the first time we see this

break; // stop when none left
continue; // silently ignore other errors }
}

std::string parent_uid;
Expand Down
Loading