s3_auth: Clear handling TSAction in the config_reloader #10556
Conversation
plugins/s3_auth/s3_auth.cc
Outdated
| Dbg(dbg_ctl, "reloading configs"); | ||
| S3Config *s3 = static_cast<S3Config *>(TSContDataGet(cont)); | ||
| S3Config *s3 = static_cast<S3Config *>(TSContDataGet(cont)); | ||
| std::unique_lock lock(s3->reload_mutex); |
There was a problem hiding this comment.
It looks like this function is calling many S3Config functions that modifies the s3 object, so acquiring the reload_mutex in the beginning seems better.
There was a problem hiding this comment.
This is a shared mutex. It only protects the data changed by copy_changes_from().
There was a problem hiding this comment.
Oh! I thought this is regular mutex...it'll be not simple fix. I'll rollback this change and let races go for now.
There was a problem hiding this comment.
For the race, @moonchen is writing up the issue and approaches.
|
[approve ci autest] |
| S3Config *s3 = static_cast<S3Config *>(TSContDataGet(cont)); | ||
| S3Config *s3 = static_cast<S3Config *>(TSContDataGet(cont)); | ||
| std::unique_lock lock(s3->reload_mutex); | ||
| s3->check_current_action(edata); |
There was a problem hiding this comment.
Why did you move this call to be first?
There was a problem hiding this comment.
This is the main change. Prior to this change, the handling action is not cleared if the config is invalid ( returning without clearing the _conf_rld_action in line 1061 )
It makes S3Config holing the invalid pointer and try to cancel it on destructor.
|
Cherry-picked to v9.2.x This has a trivial merge conflict due to TSDebug vs Dbg, which I resolved. |
(cherry picked from commit baeaf9a) Conflicts: plugins/s3_auth/s3_auth.cc
(cherry picked from commit baeaf9a) Conflicts: plugins/s3_auth/s3_auth.cc
apache#675) (cherry picked from commit baeaf9a) Conflicts: plugins/s3_auth/s3_auth.cc (cherry picked from commit 4afda66) Co-authored-by: Masaori Koshiba <masaori@apache.org>
Fix #10555.
Prior to the change, when s3_auth plugin fall into the error with invalid config file, the handling
TSAction _conf_rld_actis not cleared.