Skip to content

take stat_lock during SharedMemoryHashSet construction, since it reads shared-memory#2424

Merged
mattklein123 merged 3 commits intoenvoyproxy:masterfrom
jmarantz:shm-set-lock-on-construction
Jan 22, 2018
Merged

take stat_lock during SharedMemoryHashSet construction, since it reads shared-memory#2424
mattklein123 merged 3 commits intoenvoyproxy:masterfrom
jmarantz:shm-set-lock-on-construction

Conversation

@jmarantz
Copy link
Copy Markdown
Contributor

During construction, when attaching to an already-existing shared-memory segment, we call sanityCheck(), which reads the data model fully. So we need to hold a lock.

Signed-off-by: Joshua Marantz jmarantz@google.com

Description:
Hold the SharedMemoryHashSet in a std::unique_ptr in HotRestartImpl because it needs to take a lock during construction. We do this by moving the construction of the SharedMemoryHashSet from HotRestartImpl's initializer-list to the constructor body, holding the lock during construction.

Risk Level: Low

Testing:
//test/...

…s sanityCheck(), which reads the data model fully.

Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

LGTM, small typo, thanks

log_lock_(shmem_.log_lock_), access_log_lock_(shmem_.access_log_lock_),
stat_lock_(shmem_.stat_lock_), init_lock_(shmem_.init_lock_) {
{
// We must hold the stat lock when attaching to an existing shared-memorh segment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

typo "memorh"

Signed-off-by: Joshua Marantz <jmarantz@google.com>
@mattklein123 mattklein123 merged commit db7640b into envoyproxy:master Jan 22, 2018
@jmarantz jmarantz deleted the shm-set-lock-on-construction branch January 22, 2018 19:53
Shikugawa pushed a commit to Shikugawa/envoy that referenced this pull request Mar 28, 2020
* jwt: extract claim with whitespce as list

* fix format
jpsim pushed a commit that referenced this pull request Nov 28, 2022
Description: Add an arm64 run configuration for Envoy example app so that it's possible to run and debug Envoy Mobile example app on ARM machines (i.e., M1 Macbooks). Following steps from https://envoy-mobile.github.io/docs/envoy-mobile/latest/development/debugging/android_local.html#adding-envoy-mobile-project-into-android-studio it's possible to set up an Android project that makes it possible to run and debug Envoy Mobile example app (put symbolic breakpoints in Envoy code). As it is now, the configured/added run configuration supports running of x86 builds only which do not work on ARM machines.
Risk Level: None
Testing: Run new configuration locally on M1 Macbook.
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this pull request Nov 29, 2022
Description: Add an arm64 run configuration for Envoy example app so that it's possible to run and debug Envoy Mobile example app on ARM machines (i.e., M1 Macbooks). Following steps from https://envoy-mobile.github.io/docs/envoy-mobile/latest/development/debugging/android_local.html#adding-envoy-mobile-project-into-android-studio it's possible to set up an Android project that makes it possible to run and debug Envoy Mobile example app (put symbolic breakpoints in Envoy code). As it is now, the configured/added run configuration supports running of x86 builds only which do not work on ARM machines.
Risk Level: None
Testing: Run new configuration locally on M1 Macbook.
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
Signed-off-by: JP Simard <jp@jpsim.com>
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.

3 participants