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

value_ptr_ issue when copying/moving hardware_interface::Handle #2010

Open
kimsh5904 opened this issue Jan 21, 2025 · 3 comments · May be fixed by #2011
Open

value_ptr_ issue when copying/moving hardware_interface::Handle #2010

kimsh5904 opened this issue Jan 21, 2025 · 3 comments · May be fixed by #2011
Labels

Comments

@kimsh5904
Copy link

Describe the bug

  • It seems value_ptr_ points the same value_ after calling the below operators

Handle(const Handle & other) noexcept
{
std::unique_lock<std::shared_mutex> lock(other.handle_mutex_);
std::unique_lock<std::shared_mutex> lock_this(handle_mutex_);
prefix_name_ = other.prefix_name_;
interface_name_ = other.interface_name_;
handle_name_ = other.handle_name_;
value_ = other.value_;
value_ptr_ = other.value_ptr_;
}
Handle(Handle && other) noexcept
{
std::unique_lock<std::shared_mutex> lock(other.handle_mutex_);
std::unique_lock<std::shared_mutex> lock_this(handle_mutex_);
prefix_name_ = std::move(other.prefix_name_);
interface_name_ = std::move(other.interface_name_);
handle_name_ = std::move(other.handle_name_);
value_ = std::move(other.value_);
value_ptr_ = std::move(other.value_ptr_);
}
Handle & operator=(const Handle & other)
{
if (this != &other)
{
std::unique_lock<std::shared_mutex> lock(other.handle_mutex_);
std::unique_lock<std::shared_mutex> lock_this(handle_mutex_);
prefix_name_ = other.prefix_name_;
interface_name_ = other.interface_name_;
handle_name_ = other.handle_name_;
value_ = other.value_;
value_ptr_ = other.value_ptr_;
}
return *this;
}
Handle & operator=(Handle && other)
{
if (this != &other)
{
std::unique_lock<std::shared_mutex> lock(other.handle_mutex_);
std::unique_lock<std::shared_mutex> lock_this(handle_mutex_);
prefix_name_ = std::move(other.prefix_name_);
interface_name_ = std::move(other.interface_name_);
handle_name_ = std::move(other.handle_name_);
value_ = std::move(other.value_);
value_ptr_ = std::move(other.value_ptr_);
}
return *this;
}

@kimsh5904 kimsh5904 added the bug label Jan 21, 2025
@saikishor
Copy link
Member

saikishor commented Jan 21, 2025

Hello @kimsh5904,

Thank you for reporting the issue. Let me check and get back to you. If you already have a minimal example, it already helps to reproduce it.

Thank you!

@Wiktor-99
Copy link
Contributor

Wiktor-99 commented Jan 21, 2025

I think what we can see here is wrong ptr copy, I'm not sure but maybe we wold like to do something like this:

Handle(const Handle & other) noexcept 
 { 
   std::unique_lock<std::shared_mutex> lock(other.handle_mutex_); 
   std::unique_lock<std::shared_mutex> lock_this(handle_mutex_); 
   prefix_name_ = other.prefix_name_; 
   interface_name_ = other.interface_name_; 
   handle_name_ = other.handle_name_; 
   value_ = other.value_; 
   value_ptr_ = std::get_if<double>(&value_);
 } 

We would like copy only the value but the pointer should still point to our member value_ not to member of the other class

@saikishor
Copy link
Member

I've added some tests here: #2011

The changes look good to me. See if you can find some issues in the tests. I'm happy to check back and fix them.

Thank you for your time guys! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants