ywkaras commented on a change in pull request #7281:
URL: https://github.com/apache/trafficserver/pull/7281#discussion_r524326664
##########
File path: plugins/s3_auth/s3_auth.cc
##########
@@ -496,40 +598,56 @@ ConfigCache::get(const char *fname)
auto it = _cache.find(config_fname);
if (it != _cache.end()) {
- if (tv.tv_sec > (it->second.second + _ttl)) {
- // Update the cached configuration file.
- S3Config *s3 = new S3Config(false); // false == this config does not get
the continuation
-
- TSDebug(PLUGIN_NAME, "Configuration from %s is stale, reloading",
config_fname.c_str());
- it->second.second = tv.tv_sec;
- if (s3->parse_config(config_fname)) {
- it->second.first = s3;
+ if (tv.tv_sec > (it->second.load_time + _ttl)) {
+ unsigned update_status = it->second.update_status;
Review comment:
One thread could execute line 601 and see the condition to be true, but
then get. preempted. A second thread could execute lines 601 through 621, then
get preempted. The first thread could then resume and run through 621. When
the second thread resumed, s3, containing the function return value would be
pointing to a deleted S3Config instance. The fix is to move line 602, the
reading of update_status to before line 601.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
