You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2020/05/27 21:24:43 UTC
[GitHub] [trafficserver] bneradt opened a new pull request #6829: traffic_dump: debug_tag and lock improvements
bneradt opened a new pull request #6829:
URL: https://github.com/apache/trafficserver/pull/6829
These are changes from a set of suggestions that Walt Karas made
internally. Incidentally, these changes to disk_io_mutex likely
addresses a crash we saw for the mutex when it was acccidentally double
freed.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #6829: traffic_dump: debug_tag and lock improvements
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #6829:
URL: https://github.com/apache/trafficserver/pull/6829#discussion_r431477835
##########
File path: plugins/experimental/traffic_dump/session_data.h
##########
@@ -61,9 +62,12 @@ class SessionData
/// Whether the first transaction in this session has been written.
bool has_written_first_transaction = false;
- TSCont aio_cont = nullptr; /// AIO continuation callback
- TSCont txn_cont = nullptr; /// Transaction continuation callback
- TSMutex disk_io_mutex = nullptr; /// AIO mutex
+ TSCont aio_cont = nullptr; /// AIO continuation callback
+ TSCont txn_cont = nullptr; /// Transaction continuation callback
+
+ // The following has to be recursive because the stack does not unwind
+ // between event invocations.
+ std::recursive_mutex disk_io_mutex; /// The mutex for guarding IO calls.
Review comment:
I think you might be able to avoid making the mutex recursive. You'd have to call the reenable function in a lambda function that you passed to the constructor of an instance of ts::PostScript (created before the lock_guard). But it's probably not worth the bother.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #6829: traffic_dump: debug_tag and lock improvements
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #6829:
URL: https://github.com/apache/trafficserver/pull/6829#discussion_r431839707
##########
File path: plugins/experimental/traffic_dump/session_data.h
##########
@@ -61,9 +62,12 @@ class SessionData
/// Whether the first transaction in this session has been written.
bool has_written_first_transaction = false;
- TSCont aio_cont = nullptr; /// AIO continuation callback
- TSCont txn_cont = nullptr; /// Transaction continuation callback
- TSMutex disk_io_mutex = nullptr; /// AIO mutex
+ TSCont aio_cont = nullptr; /// AIO continuation callback
+ TSCont txn_cont = nullptr; /// Transaction continuation callback
+
+ // The following has to be recursive because the stack does not unwind
+ // between event invocations.
+ std::recursive_mutex disk_io_mutex; /// The mutex for guarding IO calls.
Review comment:
https://github.com/apache/trafficserver/blob/0b92e7cc3a6df779c4d527e0915adfb7ed936487/plugins/xdebug/xdebug.cc#L533
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [trafficserver] zwoop commented on pull request #6829: traffic_dump: debug_tag and lock improvements
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #6829:
URL: https://github.com/apache/trafficserver/pull/6829#issuecomment-637840764
Cherry-picked to v9.0.x branch.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [trafficserver] randall merged pull request #6829: traffic_dump: debug_tag and lock improvements
Posted by GitBox <gi...@apache.org>.
randall merged pull request #6829:
URL: https://github.com/apache/trafficserver/pull/6829
----------------------------------------------------------------
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:
users@infra.apache.org