You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "jpeach (via GitHub)" <gi...@apache.org> on 2023/07/09 01:13:55 UTC

[GitHub] [trafficserver] jpeach opened a new pull request, #9975: Clean up certifier plugin debug messages.

jpeach opened a new pull request, #9975:
URL: https://github.com/apache/trafficserver/pull/9975

   The certifier plugin manually write out the function name in debug messages, and these were wrong in some cases. Clean this up to use __func__ and a more consistent message format. Remove the "t" mode from fopen call, which is a Windows-ism.


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] ywkaras commented on pull request #9975: Clean up certifier plugin debug messages.

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#issuecomment-1670006424

   @bneradt is this good now?


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] jpeach commented on pull request #9975: Clean up certifier plugin debug messages.

Posted by "jpeach (via GitHub)" <gi...@apache.org>.
jpeach commented on PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#issuecomment-1630166001

   [approve ci]


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bryancall commented on pull request #9975: Clean up certifier plugin debug messages.

Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#issuecomment-1652491884

   [approve ci]


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] ywkaras commented on a diff in pull request #9975: Clean up certifier plugin debug messages.

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#discussion_r1258481928


##########
plugins/certifier/certifier.cc:
##########
@@ -529,28 +531,33 @@ cert_retriever(TSCont contp, TSEvent event, void *edata)
   SSL_CTX *ref_ctx       = nullptr;
 
   if (servername == nullptr) {
-    TSError("[%s] cert_retriever(): No SNI available.", PLUGIN_NAME);
+    TSError("[%s] %s: no SNI available", __func__, PLUGIN_NAME);
     return TS_ERROR;
   }
+
   bool wontdo = false;
   ref_ctx     = ssl_list->lookup_and_create(servername, edata, wontdo);
+
   if (wontdo) {
-    TSDebug(PLUGIN_NAME, "cert_retriever(): Won't generate cert for %s", servername);
+    TSDebug(PLUGIN_NAME, "%s: won't generate cert for %s", __func__, servername);
     TSVConnReenable(ssl_vc);
-  } else if (nullptr == ref_ctx) {
-    // If no existing context, schedule TASK thread to generate
-    TSDebug(PLUGIN_NAME, "cert_retriever(): schedule thread to generate/retrieve cert for %s", servername);
-    TSCont schedule_cont = TSContCreate(shadow_cert_generator, TSMutexCreate());
-    TSContDataSet(schedule_cont, (void *)servername);
-    TSContScheduleOnPool(schedule_cont, 0, TS_THREAD_POOL_TASK);
-  } else {
-    // Use existing context
-    TSDebug(PLUGIN_NAME, "cert_retriever(): Reuse existing cert and context for %s", servername);
+    return TS_SUCCESS;
+  }
+
+  if (ref_ctx) {
+    // Use existing context.
+    TSDebug(PLUGIN_NAME, "%s: reusing existing cert and context for %s", __func__, servername);
     SSL_set_SSL_CTX(ssl, ref_ctx);
     TSVConnReenable(ssl_vc);
+    return TS_SUCCESS;
   }
 
-  /// For scheduled connections, the scheduled continuation will handle the reenabling
+  // If no existing context, schedule TASK thread to generate.
+  TSDebug(PLUGIN_NAME, "%s: scheduling thread to generate/retrieve cert for %s", __func__, servername);
+  TSCont schedule_cont = TSContCreate(shadow_cert_generator, TSMutexCreate());
+  TSContDataSet(schedule_cont, (void *)servername);

Review Comment:
   I don' t think you need to cast to `void *`.  In any case, C-style casts are deprecated in new code.



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] jpeach commented on a diff in pull request #9975: Clean up certifier plugin debug messages.

Posted by "jpeach (via GitHub)" <gi...@apache.org>.
jpeach commented on code in PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#discussion_r1259016511


##########
plugins/certifier/certifier.cc:
##########
@@ -529,28 +531,33 @@ cert_retriever(TSCont contp, TSEvent event, void *edata)
   SSL_CTX *ref_ctx       = nullptr;
 
   if (servername == nullptr) {
-    TSError("[%s] cert_retriever(): No SNI available.", PLUGIN_NAME);
+    TSError("[%s] %s: no SNI available", __func__, PLUGIN_NAME);
     return TS_ERROR;
   }
+
   bool wontdo = false;
   ref_ctx     = ssl_list->lookup_and_create(servername, edata, wontdo);
+
   if (wontdo) {
-    TSDebug(PLUGIN_NAME, "cert_retriever(): Won't generate cert for %s", servername);
+    TSDebug(PLUGIN_NAME, "%s: won't generate cert for %s", __func__, servername);
     TSVConnReenable(ssl_vc);
-  } else if (nullptr == ref_ctx) {
-    // If no existing context, schedule TASK thread to generate
-    TSDebug(PLUGIN_NAME, "cert_retriever(): schedule thread to generate/retrieve cert for %s", servername);
-    TSCont schedule_cont = TSContCreate(shadow_cert_generator, TSMutexCreate());
-    TSContDataSet(schedule_cont, (void *)servername);
-    TSContScheduleOnPool(schedule_cont, 0, TS_THREAD_POOL_TASK);
-  } else {
-    // Use existing context
-    TSDebug(PLUGIN_NAME, "cert_retriever(): Reuse existing cert and context for %s", servername);
+    return TS_SUCCESS;
+  }
+
+  if (ref_ctx) {
+    // Use existing context.
+    TSDebug(PLUGIN_NAME, "%s: reusing existing cert and context for %s", __func__, servername);
     SSL_set_SSL_CTX(ssl, ref_ctx);
     TSVConnReenable(ssl_vc);
+    return TS_SUCCESS;
   }
 
-  /// For scheduled connections, the scheduled continuation will handle the reenabling
+  // If no existing context, schedule TASK thread to generate.
+  TSDebug(PLUGIN_NAME, "%s: scheduling thread to generate/retrieve cert for %s", __func__, servername);
+  TSCont schedule_cont = TSContCreate(shadow_cert_generator, TSMutexCreate());

Review Comment:
   This didn't change any code. The continuation is meant to be destroyed in `shadow_cert_generator`.
    



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] jpeach commented on a diff in pull request #9975: Clean up certifier plugin debug messages.

Posted by "jpeach (via GitHub)" <gi...@apache.org>.
jpeach commented on code in PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#discussion_r1257981331


##########
plugins/certifier/certifier.cc:
##########
@@ -211,7 +211,7 @@ class SslLRUList
 
         // Remove oldest node if size exceeds limit
         if (++size > limit) {
-          TSDebug(PLUGIN_NAME, "Removing %s", tail->commonName.c_str());
+          TSDebug(PLUGIN_NAME, "%s: removing '%s'", __func__, tail->commonName.c_str());

Review Comment:
   It would definitely be a separate PR.



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] jpeach commented on a diff in pull request #9975: Clean up certifier plugin debug messages.

Posted by "jpeach (via GitHub)" <gi...@apache.org>.
jpeach commented on code in PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#discussion_r1259018161


##########
plugins/certifier/certifier.cc:
##########
@@ -529,28 +531,33 @@ cert_retriever(TSCont contp, TSEvent event, void *edata)
   SSL_CTX *ref_ctx       = nullptr;
 
   if (servername == nullptr) {
-    TSError("[%s] cert_retriever(): No SNI available.", PLUGIN_NAME);
+    TSError("[%s] %s: no SNI available", __func__, PLUGIN_NAME);
     return TS_ERROR;
   }
+
   bool wontdo = false;
   ref_ctx     = ssl_list->lookup_and_create(servername, edata, wontdo);
+
   if (wontdo) {
-    TSDebug(PLUGIN_NAME, "cert_retriever(): Won't generate cert for %s", servername);
+    TSDebug(PLUGIN_NAME, "%s: won't generate cert for %s", __func__, servername);
     TSVConnReenable(ssl_vc);
-  } else if (nullptr == ref_ctx) {
-    // If no existing context, schedule TASK thread to generate
-    TSDebug(PLUGIN_NAME, "cert_retriever(): schedule thread to generate/retrieve cert for %s", servername);
-    TSCont schedule_cont = TSContCreate(shadow_cert_generator, TSMutexCreate());
-    TSContDataSet(schedule_cont, (void *)servername);
-    TSContScheduleOnPool(schedule_cont, 0, TS_THREAD_POOL_TASK);
-  } else {
-    // Use existing context
-    TSDebug(PLUGIN_NAME, "cert_retriever(): Reuse existing cert and context for %s", servername);
+    return TS_SUCCESS;
+  }
+
+  if (ref_ctx) {
+    // Use existing context.
+    TSDebug(PLUGIN_NAME, "%s: reusing existing cert and context for %s", __func__, servername);
     SSL_set_SSL_CTX(ssl, ref_ctx);
     TSVConnReenable(ssl_vc);
+    return TS_SUCCESS;
   }
 
-  /// For scheduled connections, the scheduled continuation will handle the reenabling
+  // If no existing context, schedule TASK thread to generate.
+  TSDebug(PLUGIN_NAME, "%s: scheduling thread to generate/retrieve cert for %s", __func__, servername);
+  TSCont schedule_cont = TSContCreate(shadow_cert_generator, TSMutexCreate());
+  TSContDataSet(schedule_cont, (void *)servername);

Review Comment:
   This was in the original code. Fixed.



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bneradt commented on a diff in pull request #9975: Clean up certifier plugin debug messages.

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#discussion_r1257401788


##########
plugins/certifier/certifier.cc:
##########
@@ -211,7 +211,7 @@ class SslLRUList
 
         // Remove oldest node if size exceeds limit
         if (++size > limit) {
-          TSDebug(PLUGIN_NAME, "Removing %s", tail->commonName.c_str());
+          TSDebug(PLUGIN_NAME, "%s: removing '%s'", __func__, tail->commonName.c_str());

Review Comment:
   Ah, OK. I'll leave it up to you then whether you want to keep the `__func__` here or add TSDebug support for it in a separate PR.
   
   Thanks for looking into that.



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bneradt commented on a diff in pull request #9975: Clean up certifier plugin debug messages.

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#discussion_r1257396130


##########
plugins/certifier/certifier.cc:
##########
@@ -211,7 +211,7 @@ class SslLRUList
 
         // Remove oldest node if size exceeds limit
         if (++size > limit) {
-          TSDebug(PLUGIN_NAME, "Removing %s", tail->commonName.c_str());
+          TSDebug(PLUGIN_NAME, "%s: removing '%s'", __func__, tail->commonName.c_str());

Review Comment:
   Let's definitely remove explicit function names, but I don't think we need to add them to the log via `__func__` either since we support automatically adding them:
   https://docs.trafficserver.apache.org/en/latest/admin-guide/files/records.yaml.en.html#proxy-config-diags-show-location
   
   Or does that not apply to plugin messages?



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] jpeach commented on a diff in pull request #9975: Clean up certifier plugin debug messages.

Posted by "jpeach (via GitHub)" <gi...@apache.org>.
jpeach commented on code in PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#discussion_r1257398177


##########
plugins/certifier/certifier.cc:
##########
@@ -211,7 +211,7 @@ class SslLRUList
 
         // Remove oldest node if size exceeds limit
         if (++size > limit) {
-          TSDebug(PLUGIN_NAME, "Removing %s", tail->commonName.c_str());
+          TSDebug(PLUGIN_NAME, "%s: removing '%s'", __func__, tail->commonName.c_str());

Review Comment:
   Yeh `TSDebug` doesn't know the source location. It wouldn't be too hard to fix that though.
   ```C
   tsapi void TSDebug(const char *tag, const char *format_str, ...) TS_PRINTFLIKE(2, 3);
   ```



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] jpeach commented on a diff in pull request #9975: Clean up certifier plugin debug messages.

Posted by "jpeach (via GitHub)" <gi...@apache.org>.
jpeach commented on code in PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#discussion_r1259016511


##########
plugins/certifier/certifier.cc:
##########
@@ -529,28 +531,33 @@ cert_retriever(TSCont contp, TSEvent event, void *edata)
   SSL_CTX *ref_ctx       = nullptr;
 
   if (servername == nullptr) {
-    TSError("[%s] cert_retriever(): No SNI available.", PLUGIN_NAME);
+    TSError("[%s] %s: no SNI available", __func__, PLUGIN_NAME);
     return TS_ERROR;
   }
+
   bool wontdo = false;
   ref_ctx     = ssl_list->lookup_and_create(servername, edata, wontdo);
+
   if (wontdo) {
-    TSDebug(PLUGIN_NAME, "cert_retriever(): Won't generate cert for %s", servername);
+    TSDebug(PLUGIN_NAME, "%s: won't generate cert for %s", __func__, servername);
     TSVConnReenable(ssl_vc);
-  } else if (nullptr == ref_ctx) {
-    // If no existing context, schedule TASK thread to generate
-    TSDebug(PLUGIN_NAME, "cert_retriever(): schedule thread to generate/retrieve cert for %s", servername);
-    TSCont schedule_cont = TSContCreate(shadow_cert_generator, TSMutexCreate());
-    TSContDataSet(schedule_cont, (void *)servername);
-    TSContScheduleOnPool(schedule_cont, 0, TS_THREAD_POOL_TASK);
-  } else {
-    // Use existing context
-    TSDebug(PLUGIN_NAME, "cert_retriever(): Reuse existing cert and context for %s", servername);
+    return TS_SUCCESS;
+  }
+
+  if (ref_ctx) {
+    // Use existing context.
+    TSDebug(PLUGIN_NAME, "%s: reusing existing cert and context for %s", __func__, servername);
     SSL_set_SSL_CTX(ssl, ref_ctx);
     TSVConnReenable(ssl_vc);
+    return TS_SUCCESS;
   }
 
-  /// For scheduled connections, the scheduled continuation will handle the reenabling
+  // If no existing context, schedule TASK thread to generate.
+  TSDebug(PLUGIN_NAME, "%s: scheduling thread to generate/retrieve cert for %s", __func__, servername);
+  TSCont schedule_cont = TSContCreate(shadow_cert_generator, TSMutexCreate());

Review Comment:
   This didn't change any code. Why clean up continuations on exit? Just for leak detection?
    



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] jpeach commented on pull request #9975: Clean up certifier plugin debug messages.

Posted by "jpeach (via GitHub)" <gi...@apache.org>.
jpeach commented on PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#issuecomment-1628689393

   [approve ci]


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] lzx404243 commented on a diff in pull request #9975: Clean up certifier plugin debug messages.

Posted by "lzx404243 (via GitHub)" <gi...@apache.org>.
lzx404243 commented on code in PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#discussion_r1258400214


##########
plugins/certifier/certifier.cc:
##########
@@ -419,25 +421,25 @@ shadow_cert_generator(TSCont contp, TSEvent event, void *edata)
     /// Try open the file if directory exists
     fp = fopen(cert_filename.c_str(), "rt");
   }
-  TSDebug(PLUGIN_NAME, "shadow_cert_generator(): Cert file is expected at %s", cert_filename.c_str());
+  TSDebug(PLUGIN_NAME, "%s: cert file is expected at %s", __func__, cert_filename.c_str());
   /// If cert file exists and is readable
   if (fp != nullptr) {
     cert.reset(PEM_read_X509(fp, nullptr, nullptr, nullptr));
     fclose(fp);
 
     if (cert == nullptr) {
       /// Problem with cert file / openssl read
-      TSError("[%s] [shadow_cert_generator] Problem with loading certs", PLUGIN_NAME);
+      TSError("[%s] %s: failed to loading certificated from '%s'", PLUGIN_NAME, __func__, cert_filename.c_str());

Review Comment:
   `failed to loading certificated` -> `failed to load certificate`



##########
plugins/certifier/certifier.cc:
##########
@@ -419,25 +421,25 @@ shadow_cert_generator(TSCont contp, TSEvent event, void *edata)
     /// Try open the file if directory exists
     fp = fopen(cert_filename.c_str(), "rt");
   }
-  TSDebug(PLUGIN_NAME, "shadow_cert_generator(): Cert file is expected at %s", cert_filename.c_str());
+  TSDebug(PLUGIN_NAME, "%s: cert file is expected at %s", __func__, cert_filename.c_str());
   /// If cert file exists and is readable
   if (fp != nullptr) {
     cert.reset(PEM_read_X509(fp, nullptr, nullptr, nullptr));
     fclose(fp);
 
     if (cert == nullptr) {
       /// Problem with cert file / openssl read
-      TSError("[%s] [shadow_cert_generator] Problem with loading certs", PLUGIN_NAME);
+      TSError("[%s] %s: failed to loading certificated from '%s'", PLUGIN_NAME, __func__, cert_filename.c_str());
       std::remove(cert_filename.c_str());
     } else {
-      TSDebug(PLUGIN_NAME, "shadow_cert_generator(): Loaded cert from file");
+      TSDebug(PLUGIN_NAME, "%s: loaded certificates from '%s'", __func__, cert_filename.c_str());

Review Comment:
   I think here we are loading a single certificate so `certificates` -> `certificate`



##########
tests/gold_tests/pluginTest/certifier/certifier.test.py:
##########


Review Comment:
   Looks like the following expectation needs to be updated too:
   ```
          self.ts.Disk.traffic_out.Content += Testers.ContainsExpression(
               "Creating shadow certs",
               "Verify the certifier plugin generates the certificate dynamically.")
   ```



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] bneradt commented on pull request #9975: Clean up certifier plugin debug messages.

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#issuecomment-1627569456

   Looks like the autest might need an expected log expression update:
   https://ci.trafficserver.apache.org/job/Github_Builds/job/autest/9715/console
   
   ```
        file /tmp/sandbox/certifier/ts2/log/traffic.out : Should reuse the existing certificate - Failed
           Reason: Contents of /tmp/sandbox/certifier/ts2/log/traffic.out did not contains expression: "Reuse existing cert and context for www.tls.com"
   ```


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] ywkaras commented on a diff in pull request #9975: Clean up certifier plugin debug messages.

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on code in PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#discussion_r1258488392


##########
plugins/certifier/certifier.cc:
##########
@@ -529,28 +531,33 @@ cert_retriever(TSCont contp, TSEvent event, void *edata)
   SSL_CTX *ref_ctx       = nullptr;
 
   if (servername == nullptr) {
-    TSError("[%s] cert_retriever(): No SNI available.", PLUGIN_NAME);
+    TSError("[%s] %s: no SNI available", __func__, PLUGIN_NAME);
     return TS_ERROR;
   }
+
   bool wontdo = false;
   ref_ctx     = ssl_list->lookup_and_create(servername, edata, wontdo);
+
   if (wontdo) {
-    TSDebug(PLUGIN_NAME, "cert_retriever(): Won't generate cert for %s", servername);
+    TSDebug(PLUGIN_NAME, "%s: won't generate cert for %s", __func__, servername);
     TSVConnReenable(ssl_vc);
-  } else if (nullptr == ref_ctx) {
-    // If no existing context, schedule TASK thread to generate
-    TSDebug(PLUGIN_NAME, "cert_retriever(): schedule thread to generate/retrieve cert for %s", servername);
-    TSCont schedule_cont = TSContCreate(shadow_cert_generator, TSMutexCreate());
-    TSContDataSet(schedule_cont, (void *)servername);
-    TSContScheduleOnPool(schedule_cont, 0, TS_THREAD_POOL_TASK);
-  } else {
-    // Use existing context
-    TSDebug(PLUGIN_NAME, "cert_retriever(): Reuse existing cert and context for %s", servername);
+    return TS_SUCCESS;
+  }
+
+  if (ref_ctx) {
+    // Use existing context.
+    TSDebug(PLUGIN_NAME, "%s: reusing existing cert and context for %s", __func__, servername);
     SSL_set_SSL_CTX(ssl, ref_ctx);
     TSVConnReenable(ssl_vc);
+    return TS_SUCCESS;
   }
 
-  /// For scheduled connections, the scheduled continuation will handle the reenabling
+  // If no existing context, schedule TASK thread to generate.
+  TSDebug(PLUGIN_NAME, "%s: scheduling thread to generate/retrieve cert for %s", __func__, servername);
+  TSCont schedule_cont = TSContCreate(shadow_cert_generator, TSMutexCreate());

Review Comment:
   Why not include `<tscpp/api/Cleanup.h>` and change this to:
   ```
     atscppapi::TSContUniqPtr schedule_cont{TSContCreate(shadow_cert_generator, TSMutexCreate());
   ```
   So it will be cleaned up on program exit.



-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] ywkaras commented on pull request #9975: Clean up certifier plugin debug messages.

Posted by "ywkaras (via GitHub)" <gi...@apache.org>.
ywkaras commented on PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975#issuecomment-1669965521

   [approve ci autest]


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficserver] jpeach merged pull request #9975: Clean up certifier plugin debug messages.

Posted by "jpeach (via GitHub)" <gi...@apache.org>.
jpeach merged PR #9975:
URL: https://github.com/apache/trafficserver/pull/9975


-- 
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: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org