You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "bneradt (via GitHub)" <gi...@apache.org> on 2023/10/27 22:43:19 UTC

[PR] Update ACCEPT thread configurations [trafficserver]

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

   Before this change, ACCEPT NetHandler instances would not have their configurations updated, leaving them at their initialized defaults (usually 0). This should fix things for get_additional_accepts calls.


-- 
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


Re: [PR] Update ACCEPT thread configurations [trafficserver]

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

   [approve ci centos]


-- 
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


Re: [PR] Update ACCEPT thread configurations [trafficserver]

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


##########
src/iocore/net/UnixNetAccept.cc:
##########
@@ -571,8 +571,10 @@ NetAccept::acceptLoopEvent(int event, Event *e)
 {
   (void)event;
   (void)e;
-  EThread *t = this_ethread();
-
+  EThread *t    = this_ethread();
+  NetHandler *h = get_NetHandler(t);

Review Comment:
   Interesting. Is this line a problem already then?
   https://github.com/apache/trafficserver/blob/fc3f12e5eb360a32845ddbf3cba64294c8cb1ddf/src/iocore/net/UnixNetAccept.cc#L58
   
   There's no verification at that point that the thread is a NET type. There is logic later in the function to verify NET threads, but only conditionally:
   https://github.com/apache/trafficserver/blob/fc3f12e5eb360a32845ddbf3cba64294c8cb1ddf/src/iocore/net/UnixNetAccept.cc#L114



-- 
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


Re: [PR] Update ACCEPT thread configurations [trafficserver]

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


-- 
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


Re: [PR] Update ACCEPT thread configurations [trafficserver]

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


##########
src/iocore/net/UnixNetAccept.cc:
##########
@@ -571,8 +571,10 @@ NetAccept::acceptLoopEvent(int event, Event *e)
 {
   (void)event;
   (void)e;
-  EThread *t = this_ethread();
-
+  EThread *t    = this_ethread();
+  NetHandler *h = get_NetHandler(t);

Review Comment:
   Since you're on a dedicated accept thread, and NetHandler is only initialized on ET_NET threads, the pointer h now points to an uninitialized NetHandler object.  (The memory belongs to PER_THREAD_DATA, and is actually zeroed in EThread::EThread)
   
   My suggestion would be to store this config in a thread-local block of memory, or to have the dedicated accept threads read the config values from the global config, if the config is not expected to change often.



-- 
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