ywkaras commented on code in PR #11653:
URL: https://github.com/apache/trafficserver/pull/11653#discussion_r1705671245
##########
plugins/header_rewrite/header_rewrite.cc:
##########
@@ -42,8 +42,27 @@ namespace header_rewrite_ns
DbgCtl dbg_ctl{PLUGIN_NAME_DBG};
DbgCtl pi_dbg_ctl{PLUGIN_NAME};
-std::once_flag initHRWLibs;
-PluginFactory plugin_factory;
+std::once_flag initHRWLibs;
+PluginFactory plugin_factory;
+std::atomic<int> defer{0};
+
+void
+deferRuleDone(TSHttpTxn txnp)
+{
+ defer--;
+ Dbg(pi_dbg_ctl, "Removing defer rule. Count=%d", defer.load());
+ if (defer.load() == 0) {
+ Dbg(pi_dbg_ctl, "Defer rules complete. Reenabling transaction");
+ TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
+ }
+}
Review Comment:
There is a race condition here. Consider the following scenario:
Thread 1 decrements `defer` from 2 to 1.
Thread 2 decrements `defer` from 1 to 0.
Thread 1 checks `defer` and sees it's zero, calls reeenable.
Thread 2 checks `defer` and sees it's zero, makes duplicate call to reenable.
You could rewrite this function like this:
```
void
deferRuleDone(TSHttpTxn txnp)
{
Dbg(pi_dbg_ctl, "Removing defer rule. Count=%d", defer.load());
if (--defer == 0) {
Dbg(pi_dbg_ctl, "Defer rules complete. Reenabling transaction");
TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
}
}
```
The `atomic` library guarantees that increment and decrement are atomic
operations.
--
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]
