You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "warriorpaw (via GitHub)" <gi...@apache.org> on 2023/03/27 09:26:14 UTC
[PR] Fix core when ssl is enabled (brpc)
warriorpaw opened a new pull request, #2180:
URL: https://github.com/apache/brpc/pull/2180
OpenSSL does not guarantee that any of its objects can be used concurrently by multiple threads:
https://github.com/openssl/openssl/issues/2165
https://github.com/openssl/openssl/issues/20446
Related issue: #2166
And this PR is not the best solution for this core issue.
The best way is put SSL obj's read/write operate in same event loop but this changes too much of the current code structure, i'm too lazy to do it.
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix core when ssl is enabled (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on PR #2180:
URL: https://github.com/apache/brpc/pull/2180#issuecomment-1486094743
Does this affect performance?
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix core when ssl is enabled (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on PR #2180:
URL: https://github.com/apache/brpc/pull/2180#issuecomment-1510816717
LGTM
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix core when ssl is enabled (brpc)
Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright commented on PR #2180:
URL: https://github.com/apache/brpc/pull/2180#issuecomment-1526959370
LGTM
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix core when ssl is enabled (brpc)
Posted by "warriorpaw (via GitHub)" <gi...@apache.org>.
warriorpaw commented on PR #2180:
URL: https://github.com/apache/brpc/pull/2180#issuecomment-1486138968
> Does this affect performance?
Brpc already piped write requests by linked list and only one bthread calls ssl_write.
In data reading, brpc use epoll in edge triggered mode and an atomic variable to make sure there is only one bthread is calling ssl_read.
Adding a mutex here will introduce some competition, but it should be very slight. Further this mutex will avoided the competition of using underlay socket fd. So, the impact of this change on performance should be almost none.
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix core when ssl is enabled (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm merged PR #2180:
URL: https://github.com/apache/brpc/pull/2180
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix core when ssl is enabled (brpc)
Posted by "warriorpaw (via GitHub)" <gi...@apache.org>.
warriorpaw commented on PR #2180:
URL: https://github.com/apache/brpc/pull/2180#issuecomment-1526908464
@jamesge @Huixxi @wwbmmm @chenBright
Please take another look if this change can be merged, thanks!
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Fix core when ssl is enabled (brpc)
Posted by "jingshi-ant (via GitHub)" <gi...@apache.org>.
jingshi-ant commented on PR #2180:
URL: https://github.com/apache/brpc/pull/2180#issuecomment-1486100599
> Does this affect performance?
about 20% slower in my 100w data set test. (mainly because of ssl enable. if remove lock and enable ssl, the speed is almost the same with lock added.)
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org