bneradt commented on code in PR #13432:
URL: https://github.com/apache/trafficserver/pull/13432#discussion_r3677917816
##########
plugins/healthchecks/healthchecks.cc:
##########
@@ -53,66 +54,90 @@ typedef struct HCDirEntry_t {
struct HCDirEntry_t *_next; /* Linked list */
} HCDirEntry;
-/* Information about a status file. This is never modified (only replaced, see
HCFileInfo_t) */
-typedef struct HCFileData_t {
- int exists; /* Does this file exist */
- char body[MAX_BODY_LEN]; /* Body from fname. Empty string
means file is missing */
- int b_len; /* Length of data */
- time_t remove; /* Used for deciding when the old
object can be permanently removed */
- struct HCFileData_t *_next; /* Only used when these guys end up
on the freelist */
-} HCFileData;
-
-/* The only thing that should change in this struct is data, atomically
swapping ptrs */
-typedef struct HCFileInfo_t {
- char fname[MAX_PATH_LEN]; /* Filename */
- char *basename; /* The "basename" of the file
*/
- unsigned basename_len = 0; /* The length of the basename
*/
- char path[PATH_NAME_MAX]; /* URL path for this HC */
- int p_len; /* Length of path */
- const char *ok; /* Header for an OK result */
- int o_len; /* Length of OK header */
- const char *miss; /* Header for miss results */
- int m_len; /* Length of miss header */
- std::atomic<HCFileData *> data; /* Holds the current data for
this health check file */
- int wd; /* Watch descriptor */
- HCDirEntry *dir; /* Reference to the directory
this file resides in */
- struct HCFileInfo_t *_next; /* Linked list */
-} HCFileInfo;
+/* Information about a status file. This is never modified (only replaced, see
HCFileInfo) */
+struct HCFileData {
+ int exists = 0; /* Does this file exist */
+ int b_len = 0; /* Length of data */
+ char body[MAX_BODY_LEN] = {}; /* Body from fname. Empty string means file is
missing */
+};
+
+using HCFileDataPtr = std::shared_ptr<HCFileData>;
+
+/* The only thing that should change in this struct is data, which is replaced
(never modified) by
+ the inotify thread. Readers take a reference to the current data via
get_data(), which keeps
+ that snapshot alive for as long as the transaction needs it. */
+struct HCFileInfo {
+ char fname[MAX_PATH_LEN] = {}; /* Filename */
+ char *basename = nullptr; /* The "basename" of the file */
+ unsigned basename_len = 0; /* The length of the basename */
+ char path[PATH_NAME_MAX] = {}; /* URL path for this HC */
+ int p_len = 0; /* Length of path */
+ const char *ok = nullptr; /* Header for an OK result */
+ int o_len = 0; /* Length of OK header */
+ const char *miss = nullptr; /* Header for miss results */
+ int m_len = 0; /* Length of miss header */
+ int wd = 0; /* Watch descriptor */
+ HCDirEntry *dir = nullptr; /* Reference to the directory
this file resides in */
+ HCFileInfo *_next = nullptr; /* Linked list */
+
+ /* Take a reference to the current data for this health check file. */
+ HCFileDataPtr
+ get_data()
+ {
+ std::lock_guard<std::mutex> lock{_data_mutex};
+ return _data;
+ }
+
+ /* Replace the current data for this health check file. Snapshots handed out
by get_data() stay
+ valid until their last reference is dropped. */
+ void
+ set_data(HCFileDataPtr data)
+ {
+ std::lock_guard<std::mutex> lock{_data_mutex};
+ _data = std::move(data);
+ }
+
+private:
+ std::mutex _data_mutex; /* Protects @a _data */
+ HCFileDataPtr _data; /* Holds the current data for this health check
file */
+};
Review Comment:
Replaced the request-path mutex with atomic shared-pointer operations. Newer
libraries use the C++20 specialization; GCC 11 uses the compatible atomic
load/store free functions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
