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

Allow service_discovery to continue without random_device #479

Merged
merged 3 commits into from
Nov 6, 2023

Conversation

kheaactua
Copy link
Contributor

@kheaactua kheaactua commented Jun 22, 2023

Allow service discovery to continue even if the random_device isn't yet available. This is unlikely to occur on linux systems, but not impossible. It is more likely on other systems (e.g QNX.) In the event that the random_device isn't available, an error message is produced and the middle of the min/max for the delay is used.

This PR also removes the check_routing_credentials_ member on the policy_manager_impl class when security is disabled. This removes the private error warning turned error by -Werror:

error: private field 'check_routing_credentials_' is not used [-Werror,-Wunused-private-field] error

@kheaactua
Copy link
Contributor Author

@goncaloalmeida

@kheaactua kheaactua force-pushed the missing_random_device branch 7 times, most recently from 066853d to 63bb72b Compare June 22, 2023 16:51
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @kheaactua ,
We agree with the try-catch method, but we would suggest in the fallback to use std::chrono::time_point::time_since_epoch() instead, to seed the Mersenne Twister.
That way will still ensure a decent level of randomness.
Otherwise, the PR seems ok to be merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I'll give it a try and re-push.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @kheaactua, did you had the opportunity to change the implementation to what @fcmonteiro suggested?
Thanks in advance!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Started, but got distracted :( , will get back to it ASAP

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My apologies for it taking so long. I just pushed a version with the Mersenne Twister. I tested this on QNX with and without random

Allow service_discovery to continue even if the random_device isn't yet
available.  This is unlikely to occur on linux systems, but not
impossible.  It is more likely on other systems such as QNX.  In the
event that the random device isn't available, an error message is
produced and the middle of the min/max for the delay is used.

Without this change, if the random device is missing, the following
error is thrown:

   terminate called after throwing an instance of 'std::__1::system_error'
   what():  random_device failed to open /dev/urandom: No such file or directory
   Abort (core dumped)
…d. This removes the private error warning turned error by -Werror:

error: private field 'check_routing_credentials_' is not used [-Werror,-Wunused-private-field] error
@DiogoPedrozza DiogoPedrozza merged commit 6e40119 into COVESA:master Nov 6, 2023
1 check passed
DiogoPedrozza added a commit that referenced this pull request Nov 29, 2023
Notes:
- Fix QNX build
- Fix missing Stop Offer
- Apply extra fixes to QNX environment
- Remove key <service, instance> from the offer_commands_
- #478: deleted unused param _use_exclusive_proxy
- Fix code smell related to condition variable
- #462: Fix IPv6 Service Discovery
- Added multicast_mutex to async function that makes use of multicast socket
- #479: Allow service_discovery to continue without random_device
- Fix availability handler sending false on offer
- Set host/port in vsomeip_sec_client_t whenever possible
- Use executor_work_guard instead of io_context::work for boost v1.66 and higher
- Update Windows build
- Added profile 07 as an option for E2E protection
- ASIO: use heap-allocation to avoid using garbage data
- Fix integer underflow in server_endpoint_impl.cpp
- Cleanup prepare_stop_handlers_
- Automatic unsubscribe
- Debounce now works without routingmanagerd running
- Debouncing: Send last received value after debounce time + X
- Fix resource deadlock avoided crash
- Use closure instead of callable struct
@DiogoPedrozza DiogoPedrozza mentioned this pull request Nov 29, 2023
DiogoPedrozza added a commit that referenced this pull request Nov 29, 2023
Notes:
- Fix QNX build
- Fix missing Stop Offer
- Apply extra fixes to QNX environment
- Remove key <service, instance> from the offer_commands_
- #478: deleted unused param _use_exclusive_proxy
- Fix code smell related to condition variable
- #462: Fix IPv6 Service Discovery
- Added multicast_mutex to async function that makes use of multicast socket
- #479: Allow service_discovery to continue without random_device
- Fix availability handler sending false on offer
- Set host/port in vsomeip_sec_client_t whenever possible
- Use executor_work_guard instead of io_context::work for boost v1.66 and higher
- Update Windows build
- Added profile 07 as an option for E2E protection
- ASIO: use heap-allocation to avoid using garbage data
- Fix integer underflow in server_endpoint_impl.cpp
- Cleanup prepare_stop_handlers_
- Automatic unsubscribe
- Debounce now works without routingmanagerd running
- Debouncing: Send last received value after debounce time + X
- Fix resource deadlock avoided crash
- Use closure instead of callable struct
DiogoPedrozza added a commit that referenced this pull request Nov 29, 2023
Notes:
- Fix QNX build
- Fix missing Stop Offer
- Apply extra fixes to QNX environment
- Remove key <service, instance> from the offer_commands_
- #478: deleted unused param _use_exclusive_proxy
- Fix code smell related to condition variable
- #462: Fix IPv6 Service Discovery
- Added multicast_mutex to async function that makes use of multicast socket
- #479: Allow service_discovery to continue without random_device
- Fix availability handler sending false on offer
- Set host/port in vsomeip_sec_client_t whenever possible
- Use executor_work_guard instead of io_context::work for boost v1.66 and higher
- Update Windows build
- Added profile 07 as an option for E2E protection
- ASIO: use heap-allocation to avoid using garbage data
- Fix integer underflow in server_endpoint_impl.cpp
- Cleanup prepare_stop_handlers_
- Automatic unsubscribe
- Debounce now works without routingmanagerd running
- Debouncing: Send last received value after debounce time + X
- Fix resource deadlock avoided crash
- Use closure instead of callable struct
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants