You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by GitBox <gi...@apache.org> on 2020/12/21 01:31:31 UTC

[GitHub] [thrift] cfriedt opened a new pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

cfriedt opened a new pull request #2007:
URL: https://github.com/apache/thrift/pull/2007


   <!-- Explain the changes in the pull request below: -->
   
   The TMemoryBuffer policy is "OBSERVER" by default in a constructor where an external pointer to memory, and the size of that memory, are passed in. That scenario is for externally managed buffers.
   
   As an observer of a buffer, Thrift should only be able to read. However, previously, `buf.available_read()` would return 0 (and subsequently, `buf.read()` would fail). 
   
   I've added a test as a separate commit, so it's easy to observe the failure and fix. The current behaviour produces the following failure.
   ```
   TMemoryBufferTest.cpp(141): error: in "TMemoryBufferTest/test_observer": check N == buf.available_read() has failed [1024 != 0]
   ``` 
   With this change, Thrift applications that use Memory as a transport are able to read `OBSERVER` buffers as well (as one would expect) and the error above is mitigated.
   
   <!-- We recommend you review the checklist/tips before submitting a pull request. -->
   
   - [x] Did you create an [Apache Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket?  (not required for trivial changes)
   - [x] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
   - [x] Did you squash your changes to a single commit?  (not required, but preferred)
   - [x] Did you do your best to avoid breaking changes?  If one was needed, did you label the Jira ticket with "Breaking-Change"?
   - [ ] If your change does not involve any code, add ` [skip ci]` at the end of your pull request to free up build resources.
   
   <!--
     The Contributing Guide at:
     https://github.com/apache/thrift/blob/master/CONTRIBUTING.md
     has more details and tips for committing properly.
   -->
   


----------------------------------------------------------------
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] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-646389691


   @Jens-G - should be good now. Would you like to assign reviewers?


----------------------------------------------------------------
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] [thrift] emmenlau commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
emmenlau commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-910204090


   Ok @cfried, I'm sorry but there is indeed a problem: With your changes, there are consistent crashes on all Windows builds (MSVC and MINGW) in `processor_test`, see https://ci.appveyor.com/project/ApacheSoftwareFoundation/thrift/builds/40571399. With another very recent build from yesterday, we do not get these, see https://ci.appveyor.com/project/ApacheSoftwareFoundation/thrift/builds/40603625.
   
   You can try to rebase again on latest master and force push, just to be sure its not a random fluctuation. But if the Windows tests fail again, we will need to follow it up (or at least not merge).


-- 
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: notifications-unsubscribe@thrift.apache.org

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

[GitHub] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-911474376


   > Ok @cfried, I'm sorry but there is indeed a problem: With your changes, there are consistent crashes on all Windows builds (MSVC and MINGW) in `processor_test`, see https://ci.appveyor.com/project/ApacheSoftwareFoundation/thrift/builds/40571399. With another very recent build from yesterday, we do not get these, see https://ci.appveyor.com/project/ApacheSoftwareFoundation/thrift/builds/40603625.
   
   Odd - yeah, the logs show this:
   ```
   [00:15:37] 397: Test command: C:\projects\build\MSVC2017\x64\bin\Release\processor_test.exe
   [00:15:37] 397: Test timeout computed to be: 300
   [00:15:39] 397: Thrift: Mon Aug 30 10:33:14 2021 TNonblockingServer: Serving with 1 io threads.
   [00:15:39] 397: Thrift: Mon Aug 30 10:33:14 2021 TNonblockingServer: using libevent 2.1.8-stable method win32
   [00:15:39] 397: Thrift: Mon Aug 30 10:33:14 2021 TNonblocking: IO thread #0 registered for listen.
   [00:15:39] 397: Thrift: Mon Aug 30 10:33:14 2021 TNonblocking: IO thread #0 registered for notify.
   [00:15:39] 397: Thrift: Mon Aug 30 10:33:14 2021 TNonblockingServer: IO thread #0 entering loop...
   [00:17:43] 397: Thrift: Mon Aug 30 10:35:18 2021 TNonblockingServer: caught bad_alloc exception.
   [00:17:44] 397: Running 64 test cases...
   [00:17:46] 397/416 Test #397: processor_test ...................***Failed  128.95 sec
   ```
   
   I'm not sure where the bad_alloc exception would come from, but I'll look into it if it does not pass after rebasing.


-- 
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: notifications-unsubscribe@thrift.apache.org

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

[GitHub] [thrift] emmenlau commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
emmenlau commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-983450878


   Hi, any updates on this?


-- 
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: notifications-unsubscribe@thrift.apache.org

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

[GitHub] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-646580791


   ```console
   $ make check
   ...
   make[5]: Leaving directory '/home/cfriedt/workspace/thrift/lib/cpp/test'
   make  check-TESTS
   make[5]: Entering directory '/home/cfriedt/workspace/thrift/lib/cpp/test'
   An error message from getaddrinfo on the console is expected:
   Thrift: Fri Jun 19 07:12:34 2020 getaddrinfo() -> -2; Name or service not known
   
   *** No errors detected
   PASS: UnitTests
   Thrift: Fri Jun 19 07:12:34 2020 ~TFDTransport TTransportException: 'TFDTransport::close(): Bad file descriptor'
   
   *** No errors detected
   PASS: TFDTransportTest
   
   *** No errors detected
   PASS: TPipedTransportTest
   *** No errors detected
   PASS: DebugProtoTest
   
   *** No errors detected
   PASS: JSONProtoTest
   
   *** No errors detected
   PASS: OptionalRequiredTest
   
   *** No errors detected
   PASS: RecursiveTest
   
   *** No errors detected
   PASS: SpecializationTest
   TBinaryProtocol => OK
   TLEBinaryProtocol => OK
   TCompactProtocol => OK
   
   *** No errors detected
   PASS: AllProtocolsTest
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   
   *** No errors detected
   PASS: TransportTest
   Thrift: Fri Jun 19 07:13:21 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   
   *** No errors detected
   PASS: TInterruptTest
   
   *** No errors detected
   PASS: TServerIntegrationTest
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   
   *** No errors detected
   PASS: SecurityTest
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Fri Jun 19 07:13:40 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   
   *** No errors detected
   PASS: SecurityFromBufferTest
   seed: 1592565220
   
   *** No errors detected
   PASS: ZlibTest
   
   *** No errors detected
   PASS: TFileTransportTest
   PASS: link_test
   
   *** No errors detected
   PASS: OpenSSLManualInitTest
   
   *** No errors detected
   PASS: EnumTest
   
   *** No errors detected
   PASS: RenderedDoubleConstantsTest
   
   *** No errors detected
   PASS: AnnotationTest
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: join done for IO thread #0
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: join done for IO thread #0
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: join done for IO thread #0
   
   *** No errors detected
   PASS: TNonblockingServerTest
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Fri Jun 19 07:13:43 2020 TConnection::workSocket(): client disconnected
   Thrift: Fri Jun 19 07:13:43 2020 SSL_shutdown: Broken pipe (SSL_error_code = 6)
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: join done for IO thread #0
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Fri Jun 19 07:13:43 2020 TConnection::workSocket(): client disconnected
   Thrift: Fri Jun 19 07:13:43 2020 SSL_shutdown: Broken pipe (SSL_error_code = 6)
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: join done for IO thread #0
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Fri Jun 19 07:13:43 2020 TConnection::workSocket(): client disconnected
   Thrift: Fri Jun 19 07:13:43 2020 SSL_shutdown: Broken pipe (SSL_error_code = 6)
   Thrift: Fri Jun 19 07:13:43 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Fri Jun 19 07:13:43 2020 TNonblocking: join done for IO thread #0
   
   *** No errors detected
   PASS: TNonblockingSSLServerTest
   ===================
   All 23 tests passed
   ===================
   ...
   cfriedt@cfriedt-MacBookPro:~/workspace/thrift$ echo $?
   0
   cfriedt@cfriedt-MacBookPro:~/workspace/thrift$ uname -a
   Linux cfriedt-MacBookPro 4.15.0-106-generic #107-Ubuntu SMP Thu Jun 4 11:27:52 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
   ```


----------------------------------------------------------------
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] [thrift] Jens-G commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
Jens-G commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-655089604


   @emmenlau Ping?


----------------------------------------------------------------
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] [thrift] stale[bot] commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-743934814


   This issue has been automatically closed due to inactivity. Thank you for your contributions.
   


----------------------------------------------------------------
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] [thrift] cfriedt commented on a change in pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on a change in pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#discussion_r443396568



##########
File path: lib/cpp/src/thrift/transport/TBufferTransports.h
##########
@@ -587,11 +587,9 @@ class TMemoryBuffer : public TVirtualTransport<TMemoryBuffer, TBufferBase> {
   void resetBuffer() {
     rBase_ = buffer_;
     rBound_ = buffer_;
-    wBase_ = buffer_;
-    // It isn't safe to write into a buffer we don't own.
-    if (!owner_) {
-      wBound_ = wBase_;
-      bufferSize_ = 0;
+    if (owner_) {
+      wBase_ = buffer_;
+      wBound_ = buffer_ + bufferSize_;

Review comment:
       If the buffer is not owned (is read-only, i.e. was created with `policy == OBSERVE`), `wBase_` is initialized to `wBase_ = buffer_ + wPos` (the end of the buffer) inside of `initCommon()` and never updated.
   
   Throughout the lifetime of the buffer, if it is not owned (read-only), `wBound_ == wBase_`, meaning there is no space to write.
   
   Only if the buffer is owned (writeable) should `wbase_` and `wbound_` be reset to the beginning / end of the buffer, in `resetBuffer()`.
   
   If the buffer is not owned (i.e. is read-only), it was created with `policy == OBSERVE`.




----------------------------------------------------------------
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] [thrift] stale[bot] closed pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #2007:
URL: https://github.com/apache/thrift/pull/2007


   


----------------------------------------------------------------
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] [thrift] stale[bot] commented on pull request #2007: THRIFT-5093: TMemoryBuffer constructor initializes wpos incorrectly

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-620372705


   This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.
   


----------------------------------------------------------------
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] [thrift] stale[bot] commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-748709861


   This issue is no longer stale. Thank you for your contributions.
   


----------------------------------------------------------------
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] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-646595358


   Added `test_take_ownership()` to illustrate that is still working fine.


----------------------------------------------------------------
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] [thrift] Jens-G edited a comment on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
Jens-G edited a comment on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-653682568


   @emmenlau Still not sure what the status here is ... changes requested? all fine? something else?


----------------------------------------------------------------
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] [thrift] cfriedt commented on a change in pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on a change in pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#discussion_r443396568



##########
File path: lib/cpp/src/thrift/transport/TBufferTransports.h
##########
@@ -587,11 +587,9 @@ class TMemoryBuffer : public TVirtualTransport<TMemoryBuffer, TBufferBase> {
   void resetBuffer() {
     rBase_ = buffer_;
     rBound_ = buffer_;
-    wBase_ = buffer_;
-    // It isn't safe to write into a buffer we don't own.
-    if (!owner_) {
-      wBound_ = wBase_;
-      bufferSize_ = 0;
+    if (owner_) {
+      wBase_ = buffer_;
+      wBound_ = buffer_ + bufferSize_;

Review comment:
       If the buffer is not owned (is read-only, i.e. was created with `policy == OBSERVE`), `wBase_` is initialized to `wBase_ = buffer_ + wPos` (the end of the buffer) inside of `initCommon()` and never updated.
   
   Throughout the lifetime of the buffer, if it is not owned (read-only), `wBound_ == wBase_`, meaning there is no space to write.
   
   Only if the buffer is owned (writeable) should `wbase_` and `wbound_` be reset to the beginning / end of the buffer, in `resetBuffer()`. If the buffer is not owned (i.e. is read-only, i.e. was created with `policy == OBSERVE`).




----------------------------------------------------------------
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] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-655528359


   > I could try rebasing to see if unrelated tests pass.
   
   It's already at the latest from master.


----------------------------------------------------------------
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] [thrift] emmenlau commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
emmenlau commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-908199868


   @cfriedt I've just this very second merged a PR that fixes the Travis build, please be sure to include it in your rebase if possible. Thanks a lot!


-- 
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: notifications-unsubscribe@thrift.apache.org

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

[GitHub] [thrift] emmenlau commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
emmenlau commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-744469887


   I'll try to review this again soon. Please ping me if I don't get back to it soon!


----------------------------------------------------------------
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] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-646589963


   On aarch64, completely unrelated failures (also works on x86_64, but not on aarch64). Error is specific to SSL.
   
   x86_64 machine is Ubuntu Bionic, aarch64 machine is Debian 10.2
   
   x86_64: libssl1.1 is already the newest version (1.1.1-1ubuntu2.1~18.04.6).
   aarch64: libssl1.1 is already the newest version (1.1.1d-0+deb10u3).
   
   ```console
   make[5]: Leaving directory '/home/chrisfriedt/workspace/thrift/lib/cpp/test'
   make  check-TESTS
   make[5]: Entering directory '/home/chrisfriedt/workspace/thrift/lib/cpp/test'
   Thrift: Tue Jun 16 07:16:22 2020 TSocket::open() connect() <Host: localhost Port: 46709>: Connection refused
   An error message from getaddrinfo on the console is expected:
   Thrift: Tue Jun 16 07:16:26 2020 getaddrinfo() -> -2; Name or service not known
   
   *** No errors detected
   PASS: UnitTests
   Thrift: Tue Jun 16 07:16:26 2020 ~TFDTransport TTransportException: 'TFDTransport::close(): Bad file descriptor'
   
   *** No errors detected
   PASS: TFDTransportTest
   
   *** No errors detected
   PASS: TPipedTransportTest
   
   *** No errors detected
   PASS: DebugProtoTest
   
   *** No errors detected
   PASS: JSONProtoTest
   
   *** No errors detected
   PASS: OptionalRequiredTest
   
   *** No errors detected
   PASS: RecursiveTest
   
   *** No errors detected
   PASS: SpecializationTest
   TBinaryProtocol => OK
   TLEBinaryProtocol => OK
   TCompactProtocol => OK
   
   *** No errors detected
   PASS: AllProtocolsTest
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   
   *** No errors detected
   PASS: TransportTest
   Thrift: Tue Jun 16 07:17:29 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   
   *** No errors detected
   PASS: TInterruptTest
   
   *** No errors detected
   PASS: TServerIntegrationTest
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   terminate called after throwing an instance of 'boost::execution_aborted'
   
   *** 4 failures are detected in the test module "SecurityTest"
   FAIL: SecurityTest
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   terminate called after throwing an instance of 'boost::execution_aborted'
   
   *** 4 failures are detected in the test module "SecurityFromBufferTest"
   FAIL: SecurityFromBufferTest
   seed: 1592306269
   
   *** No errors detected
   PASS: ZlibTest
   
   *** No errors detected
   PASS: TFileTransportTest
   PASS: link_test
   
   *** No errors detected
   PASS: OpenSSLManualInitTest
   *** No errors detected
   PASS: EnumTest
   
   *** No errors detected
   PASS: RenderedDoubleConstantsTest
   
   *** No errors detected
   PASS: AnnotationTest
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 12345>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 33643>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 37905>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   
   *** No errors detected
   PASS: TNonblockingServerTest
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 12345>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TConnection::workSocket(): client disconnected
   Thrift: Tue Jun 16 07:17:56 2020 SSL_shutdown: Broken pipe (SSL_error_code = 6)
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 36637>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TConnection::workSocket(): client disconnected
   Thrift: Tue Jun 16 07:17:56 2020 SSL_shutdown: Broken pipe (SSL_error_code = 6)
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 40159>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TConnection::workSocket(): client disconnected
   Thrift: Tue Jun 16 07:17:56 2020 SSL_shutdown: Broken pipe (SSL_error_code = 6)
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   
   *** No errors detected
   PASS: TNonblockingSSLServerTest
   ====================
   2 of 23 tests failed
   ====================
   
    ./SecurityTest
   Running 1 test case...
   Thrift: Tue Jun 16 07:23:33 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:23:33 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   SecurityTest.cpp(268): error: in "SecurityTest/ssl_security_matrix":       Server = SSLTLS, Client = TLSv1_0 expected mConnected == 1 but was 0
   Thrift: Tue Jun 16 07:23:33 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:23:33 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   SecurityTest.cpp(268): error: in "SecurityTest/ssl_security_matrix":       Server = SSLTLS, Client = TLSv1_1 expected mConnected == 1 but was 0
   SecurityTest.cpp(149): fatal error: in "SecurityTest/ssl_security_matrix": N6apache6thrift9transport13TSSLExceptionE: SSL_CTX_set_cipher_list: bad value
   terminate called after throwing an instance of 'boost::execution_aborted'
   unknown location(0): fatal error: in "SecurityTest/ssl_security_matrix": signal: SIGABRT (application abort requested)
   SecurityTest.cpp(149): last checkpoint
   
   *** 4 failures are detected in the test module "SecurityTest"
   
   $ ./SecurityFromBufferTest
   Running 1 test case...
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: Broken pipe (SSL_error_code = 5)
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   SecurityFromBufferTest.cpp(245): error: in "SecurityFromBufferTest/ssl_security_matrix":       Server = SSLTLS, Client = TLSv1_0 expected mConnected == 1 but was 0
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   SecurityFromBufferTest.cpp(245): error: in "SecurityFromBufferTest/ssl_security_matrix":       Server = SSLTLS, Client = TLSv1_1 expected mConnected == 1 but was 0
   SecurityFromBufferTest.cpp(145): fatal error: in "SecurityFromBufferTest/ssl_security_matrix": N6apache6thrift9transport13TSSLExceptionE: SSL_CTX_set_cipher_list: bad value
   terminate called after throwing an instance of 'boost::execution_aborted'
   unknown location(0): fatal error: in "SecurityFromBufferTest/ssl_security_matrix": signal: SIGABRT (application abort requested)
   SecurityFromBufferTest.cpp(145): last checkpoint
   $ uname -a
   Linux penguin 4.19.113-08528-g5803a1c7e9f9 #1 SMP PREEMPT Thu Apr 2 15:16:47 PDT 2020 aarch64 GNU/Linux
   ```


----------------------------------------------------------------
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] [thrift] stale[bot] commented on pull request #2007: THRIFT-5093: TMemoryBuffer constructor initializes wpos incorrectly

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-620540372


   This issue is no longer stale. Thank you for your contributions.
   


----------------------------------------------------------------
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] [thrift] emmenlau commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
emmenlau commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-908191984


   @cfriedt for the sake of a nicer git history, could you rebase on latest master?
   @all, I would merge these changes as they have worked for me successfully for a long time and there are tests that seem to confirm the improved behavior. Please speak up if there are any concerns against merging.


-- 
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: notifications-unsubscribe@thrift.apache.org

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

[GitHub] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-655446498


   I could try rebasing to see if unrelated tests pass.


----------------------------------------------------------------
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] [thrift] cfriedt removed a comment on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt removed a comment on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-646589963


   On aarch64, completely unrelated failures (also works on x86_64, but not on aarch64). Error is specific to SSL.
   
   x86_64 machine is Ubuntu Bionic, aarch64 machine is Debian 10.2
   
   x86_64: libssl1.1 is already the newest version (1.1.1-1ubuntu2.1~18.04.6).
   aarch64: libssl1.1 is already the newest version (1.1.1d-0+deb10u3).
   
   ```console
   make[5]: Leaving directory '/home/chrisfriedt/workspace/thrift/lib/cpp/test'
   make  check-TESTS
   make[5]: Entering directory '/home/chrisfriedt/workspace/thrift/lib/cpp/test'
   Thrift: Tue Jun 16 07:16:22 2020 TSocket::open() connect() <Host: localhost Port: 46709>: Connection refused
   An error message from getaddrinfo on the console is expected:
   Thrift: Tue Jun 16 07:16:26 2020 getaddrinfo() -> -2; Name or service not known
   
   *** No errors detected
   PASS: UnitTests
   Thrift: Tue Jun 16 07:16:26 2020 ~TFDTransport TTransportException: 'TFDTransport::close(): Bad file descriptor'
   
   *** No errors detected
   PASS: TFDTransportTest
   
   *** No errors detected
   PASS: TPipedTransportTest
   
   *** No errors detected
   PASS: DebugProtoTest
   
   *** No errors detected
   PASS: JSONProtoTest
   
   *** No errors detected
   PASS: OptionalRequiredTest
   
   *** No errors detected
   PASS: RecursiveTest
   
   *** No errors detected
   PASS: SpecializationTest
   TBinaryProtocol => OK
   TLEBinaryProtocol => OK
   TCompactProtocol => OK
   
   *** No errors detected
   PASS: AllProtocolsTest
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   
   *** No errors detected
   PASS: TransportTest
   Thrift: Tue Jun 16 07:17:29 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   
   *** No errors detected
   PASS: TInterruptTest
   
   *** No errors detected
   PASS: TServerIntegrationTest
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   terminate called after throwing an instance of 'boost::execution_aborted'
   
   *** 4 failures are detected in the test module "SecurityTest"
   FAIL: SecurityTest
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   terminate called after throwing an instance of 'boost::execution_aborted'
   
   *** 4 failures are detected in the test module "SecurityFromBufferTest"
   FAIL: SecurityFromBufferTest
   seed: 1592306269
   
   *** No errors detected
   PASS: ZlibTest
   
   *** No errors detected
   PASS: TFileTransportTest
   PASS: link_test
   
   *** No errors detected
   PASS: OpenSSLManualInitTest
   *** No errors detected
   PASS: EnumTest
   
   *** No errors detected
   PASS: RenderedDoubleConstantsTest
   
   *** No errors detected
   PASS: AnnotationTest
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 12345>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 33643>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 37905>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   
   *** No errors detected
   PASS: TNonblockingServerTest
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 12345>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TConnection::workSocket(): client disconnected
   Thrift: Tue Jun 16 07:17:56 2020 SSL_shutdown: Broken pipe (SSL_error_code = 6)
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 36637>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TConnection::workSocket(): client disconnected
   Thrift: Tue Jun 16 07:17:56 2020 SSL_shutdown: Broken pipe (SSL_error_code = 6)
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 40159>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TConnection::workSocket(): client disconnected
   Thrift: Tue Jun 16 07:17:56 2020 SSL_shutdown: Broken pipe (SSL_error_code = 6)
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   
   *** No errors detected
   PASS: TNonblockingSSLServerTest
   ====================
   2 of 23 tests failed
   ====================
   
    ./SecurityTest
   Running 1 test case...
   Thrift: Tue Jun 16 07:23:33 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:23:33 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   SecurityTest.cpp(268): error: in "SecurityTest/ssl_security_matrix":       Server = SSLTLS, Client = TLSv1_0 expected mConnected == 1 but was 0
   Thrift: Tue Jun 16 07:23:33 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:23:33 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   SecurityTest.cpp(268): error: in "SecurityTest/ssl_security_matrix":       Server = SSLTLS, Client = TLSv1_1 expected mConnected == 1 but was 0
   SecurityTest.cpp(149): fatal error: in "SecurityTest/ssl_security_matrix": N6apache6thrift9transport13TSSLExceptionE: SSL_CTX_set_cipher_list: bad value
   terminate called after throwing an instance of 'boost::execution_aborted'
   unknown location(0): fatal error: in "SecurityTest/ssl_security_matrix": signal: SIGABRT (application abort requested)
   SecurityTest.cpp(149): last checkpoint
   
   *** 4 failures are detected in the test module "SecurityTest"
   
   $ ./SecurityFromBufferTest
   Running 1 test case...
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: Broken pipe (SSL_error_code = 5)
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   SecurityFromBufferTest.cpp(245): error: in "SecurityFromBufferTest/ssl_security_matrix":       Server = SSLTLS, Client = TLSv1_0 expected mConnected == 1 but was 0
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   SecurityFromBufferTest.cpp(245): error: in "SecurityFromBufferTest/ssl_security_matrix":       Server = SSLTLS, Client = TLSv1_1 expected mConnected == 1 but was 0
   SecurityFromBufferTest.cpp(145): fatal error: in "SecurityFromBufferTest/ssl_security_matrix": N6apache6thrift9transport13TSSLExceptionE: SSL_CTX_set_cipher_list: bad value
   terminate called after throwing an instance of 'boost::execution_aborted'
   unknown location(0): fatal error: in "SecurityFromBufferTest/ssl_security_matrix": signal: SIGABRT (application abort requested)
   SecurityFromBufferTest.cpp(145): last checkpoint
   $ uname -a
   Linux penguin 4.19.113-08528-g5803a1c7e9f9 #1 SMP PREEMPT Thu Apr 2 15:16:47 PDT 2020 aarch64 GNU/Linux
   ```
   
   Just checked and both of those tests fail on master as well on this machine. Should be pretty safe.


----------------------------------------------------------------
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] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer constructor initializes wpos incorrectly

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-644497391


   Let me take a look again.


----------------------------------------------------------------
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] [thrift] emmenlau commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
emmenlau commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-898438129


   I've been using these changes successfully in my code for a few months now. I'm not certain that qualifies as a validation, but I personally would be open to merging this PR. Other opinions?


-- 
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: notifications-unsubscribe@thrift.apache.org

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

[GitHub] [thrift] cfriedt commented on a change in pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on a change in pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#discussion_r443396568



##########
File path: lib/cpp/src/thrift/transport/TBufferTransports.h
##########
@@ -587,11 +587,9 @@ class TMemoryBuffer : public TVirtualTransport<TMemoryBuffer, TBufferBase> {
   void resetBuffer() {
     rBase_ = buffer_;
     rBound_ = buffer_;
-    wBase_ = buffer_;
-    // It isn't safe to write into a buffer we don't own.
-    if (!owner_) {
-      wBound_ = wBase_;
-      bufferSize_ = 0;
+    if (owner_) {
+      wBase_ = buffer_;
+      wBound_ = buffer_ + bufferSize_;

Review comment:
       If the buffer is not owned (is read-only, i.e. was created with `policy == OBSERVE`), `wBase_` is initialized to `wBase_ = buffer_ + wPos` (the end of the buffer) inside of `initCommon()` and never updated.
   
   Throughout the lifetime of the buffer, if it is not owned (read-only), `wBound_ == wBase_`, meaning there is no space to write (both `wBound_` and `wBase_` continue to point at the end of the buffer).
   
   Only if the buffer is owned (writeable) should `wbase_` and `wbound_` be reset to the beginning / end of the buffer, in `resetBuffer()`.
   
   I agree, the current implementation is a bit confusing and it might be possible to rewrite it to be more obvious. This change realizes the intent of the design while keeping changes to the implementation minimal. 




----------------------------------------------------------------
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] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-840743320


   Ping, @emmenlau ;-)


-- 
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] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-983845605


   Man - I wish there were more hours in the day! Up until a few weeks ago, I only had Mac machines and it was somewhat painful to set up Windows in a VM to try and reproduce the weird Windows issues. Now I have a decent Linux machine to use for that, so I'll give it another go in the next few days.
   
   Thanks, for checking in :-)


-- 
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: notifications-unsubscribe@thrift.apache.org

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

[GitHub] [thrift] Jens-G commented on pull request #2007: THRIFT-5093: TMemoryBuffer constructor initializes wpos incorrectly

Posted by GitBox <gi...@apache.org>.
Jens-G commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-641591547


   Not sure about the state of this, also CI seems all "red". How do we proceed here?


----------------------------------------------------------------
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] [thrift] cfriedt commented on a change in pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on a change in pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#discussion_r443698582



##########
File path: lib/cpp/src/thrift/transport/TBufferTransports.h
##########
@@ -587,11 +587,9 @@ class TMemoryBuffer : public TVirtualTransport<TMemoryBuffer, TBufferBase> {
   void resetBuffer() {
     rBase_ = buffer_;
     rBound_ = buffer_;
-    wBase_ = buffer_;
-    // It isn't safe to write into a buffer we don't own.
-    if (!owner_) {
-      wBound_ = wBase_;
-      bufferSize_ = 0;
+    if (owner_) {
+      wBase_ = buffer_;
+      wBound_ = buffer_ + bufferSize_;

Review comment:
       Please let me know if you would like additional testing done to illustrate things a bit more.




----------------------------------------------------------------
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] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer constructor initializes wpos incorrectly

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-620540339


   pushing back again, sorry


----------------------------------------------------------------
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] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-646601397


   Also, this should probably be labelled as a bug and not a feature (it has no labels yet).


----------------------------------------------------------------
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] [thrift] cfriedt commented on a change in pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on a change in pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#discussion_r443396568



##########
File path: lib/cpp/src/thrift/transport/TBufferTransports.h
##########
@@ -587,11 +587,9 @@ class TMemoryBuffer : public TVirtualTransport<TMemoryBuffer, TBufferBase> {
   void resetBuffer() {
     rBase_ = buffer_;
     rBound_ = buffer_;
-    wBase_ = buffer_;
-    // It isn't safe to write into a buffer we don't own.
-    if (!owner_) {
-      wBound_ = wBase_;
-      bufferSize_ = 0;
+    if (owner_) {
+      wBase_ = buffer_;
+      wBound_ = buffer_ + bufferSize_;

Review comment:
       If the buffer is not owned (read-only), wbase is initialized to buffer + wpos (the end of the buffer) and never updated, along with wbound. Throughout the lifetime of the buffer, if it is not owned (read-only), wbase == wbound meaning there is no space to write. Only if the buffer is owned (writeable) should wbase and wbound be reset to the beginning / end of the buffer.




----------------------------------------------------------------
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] [thrift] Jens-G commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
Jens-G commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-646987173


   @emmenlau Can do do 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.

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

[GitHub] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-646595854


   Reviewers, @emmenlau  @Jens-G? 


----------------------------------------------------------------
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] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-674168678


   @Jens-G - will this be merged soon? I'd be happy to make additional changes if required.


----------------------------------------------------------------
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] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-908195674


   @emmenlau - will do


-- 
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: notifications-unsubscribe@thrift.apache.org

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

[GitHub] [thrift] Jens-G commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
Jens-G commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-653682568


   @emmenlau Still not sure what the status here is ... :-)


----------------------------------------------------------------
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] [thrift] stale[bot] commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-711120425


   This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.
   


----------------------------------------------------------------
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] [thrift] cfriedt edited a comment on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt edited a comment on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-646589963


   On aarch64, completely unrelated failures (also works on x86_64, but not on aarch64). Error is specific to SSL.
   
   x86_64 machine is Ubuntu Bionic, aarch64 machine is Debian 10.2
   
   x86_64: libssl1.1 is already the newest version (1.1.1-1ubuntu2.1~18.04.6).
   aarch64: libssl1.1 is already the newest version (1.1.1d-0+deb10u3).
   
   ```console
   make[5]: Leaving directory '/home/chrisfriedt/workspace/thrift/lib/cpp/test'
   make  check-TESTS
   make[5]: Entering directory '/home/chrisfriedt/workspace/thrift/lib/cpp/test'
   Thrift: Tue Jun 16 07:16:22 2020 TSocket::open() connect() <Host: localhost Port: 46709>: Connection refused
   An error message from getaddrinfo on the console is expected:
   Thrift: Tue Jun 16 07:16:26 2020 getaddrinfo() -> -2; Name or service not known
   
   *** No errors detected
   PASS: UnitTests
   Thrift: Tue Jun 16 07:16:26 2020 ~TFDTransport TTransportException: 'TFDTransport::close(): Bad file descriptor'
   
   *** No errors detected
   PASS: TFDTransportTest
   
   *** No errors detected
   PASS: TPipedTransportTest
   
   *** No errors detected
   PASS: DebugProtoTest
   
   *** No errors detected
   PASS: JSONProtoTest
   
   *** No errors detected
   PASS: OptionalRequiredTest
   
   *** No errors detected
   PASS: RecursiveTest
   
   *** No errors detected
   PASS: SpecializationTest
   TBinaryProtocol => OK
   TLEBinaryProtocol => OK
   TCompactProtocol => OK
   
   *** No errors detected
   PASS: AllProtocolsTest
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   Timeout alarm expired; attempting to unblock transport
   
   *** No errors detected
   PASS: TransportTest
   Thrift: Tue Jun 16 07:17:29 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   
   *** No errors detected
   PASS: TInterruptTest
   
   *** No errors detected
   PASS: TServerIntegrationTest
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   terminate called after throwing an instance of 'boost::execution_aborted'
   
   *** 4 failures are detected in the test module "SecurityTest"
   FAIL: SecurityTest
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:17:49 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   terminate called after throwing an instance of 'boost::execution_aborted'
   
   *** 4 failures are detected in the test module "SecurityFromBufferTest"
   FAIL: SecurityFromBufferTest
   seed: 1592306269
   
   *** No errors detected
   PASS: ZlibTest
   
   *** No errors detected
   PASS: TFileTransportTest
   PASS: link_test
   
   *** No errors detected
   PASS: OpenSSLManualInitTest
   *** No errors detected
   PASS: EnumTest
   
   *** No errors detected
   PASS: RenderedDoubleConstantsTest
   
   *** No errors detected
   PASS: AnnotationTest
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 12345>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 33643>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 37905>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   
   *** No errors detected
   PASS: TNonblockingServerTest
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 12345>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TConnection::workSocket(): client disconnected
   Thrift: Tue Jun 16 07:17:56 2020 SSL_shutdown: Broken pipe (SSL_error_code = 6)
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 36637>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TConnection::workSocket(): client disconnected
   Thrift: Tue Jun 16 07:17:56 2020 SSL_shutdown: Broken pipe (SSL_error_code = 6)
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: Serving with 1 io threads.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: using libevent 2.1.8-stable method epoll
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for listen.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: IO thread #0 registered for notify.
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 entering loop...
   Thrift: Tue Jun 16 07:17:56 2020 TSocket::open() connect() <Host: localhost Port: 40159>: Connection refused
   Thrift: Tue Jun 16 07:17:56 2020 TConnection::workSocket(): client disconnected
   Thrift: Tue Jun 16 07:17:56 2020 SSL_shutdown: Broken pipe (SSL_error_code = 6)
   Thrift: Tue Jun 16 07:17:56 2020 TNonblockingServer: IO thread #0 run() done!
   Thrift: Tue Jun 16 07:17:56 2020 TNonblocking: join done for IO thread #0
   
   *** No errors detected
   PASS: TNonblockingSSLServerTest
   ====================
   2 of 23 tests failed
   ====================
   
    ./SecurityTest
   Running 1 test case...
   Thrift: Tue Jun 16 07:23:33 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:23:33 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   SecurityTest.cpp(268): error: in "SecurityTest/ssl_security_matrix":       Server = SSLTLS, Client = TLSv1_0 expected mConnected == 1 but was 0
   Thrift: Tue Jun 16 07:23:33 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:23:33 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   SecurityTest.cpp(268): error: in "SecurityTest/ssl_security_matrix":       Server = SSLTLS, Client = TLSv1_1 expected mConnected == 1 but was 0
   SecurityTest.cpp(149): fatal error: in "SecurityTest/ssl_security_matrix": N6apache6thrift9transport13TSSLExceptionE: SSL_CTX_set_cipher_list: bad value
   terminate called after throwing an instance of 'boost::execution_aborted'
   unknown location(0): fatal error: in "SecurityTest/ssl_security_matrix": signal: SIGABRT (application abort requested)
   SecurityTest.cpp(149): last checkpoint
   
   *** 4 failures are detected in the test module "SecurityTest"
   
   $ ./SecurityFromBufferTest
   Running 1 test case...
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: Broken pipe (SSL_error_code = 5)
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   SecurityFromBufferTest.cpp(245): error: in "SecurityFromBufferTest/ssl_security_matrix":       Server = SSLTLS, Client = TLSv1_0 expected mConnected == 1 but was 0
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   Thrift: Tue Jun 16 07:19:42 2020 SSL_shutdown: shutdown while in init (SSL_error_code = 1)
   SecurityFromBufferTest.cpp(245): error: in "SecurityFromBufferTest/ssl_security_matrix":       Server = SSLTLS, Client = TLSv1_1 expected mConnected == 1 but was 0
   SecurityFromBufferTest.cpp(145): fatal error: in "SecurityFromBufferTest/ssl_security_matrix": N6apache6thrift9transport13TSSLExceptionE: SSL_CTX_set_cipher_list: bad value
   terminate called after throwing an instance of 'boost::execution_aborted'
   unknown location(0): fatal error: in "SecurityFromBufferTest/ssl_security_matrix": signal: SIGABRT (application abort requested)
   SecurityFromBufferTest.cpp(145): last checkpoint
   $ uname -a
   Linux penguin 4.19.113-08528-g5803a1c7e9f9 #1 SMP PREEMPT Thu Apr 2 15:16:47 PDT 2020 aarch64 GNU/Linux
   ```
   
   Just checked and both of those tests fail on master as well on this machine. Should be pretty safe.


----------------------------------------------------------------
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] [thrift] emmenlau commented on a change in pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
emmenlau commented on a change in pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#discussion_r443385854



##########
File path: lib/cpp/src/thrift/transport/TBufferTransports.h
##########
@@ -587,11 +587,9 @@ class TMemoryBuffer : public TVirtualTransport<TMemoryBuffer, TBufferBase> {
   void resetBuffer() {
     rBase_ = buffer_;
     rBound_ = buffer_;
-    wBase_ = buffer_;
-    // It isn't safe to write into a buffer we don't own.
-    if (!owner_) {
-      wBound_ = wBase_;
-      bufferSize_ = 0;
+    if (owner_) {
+      wBase_ = buffer_;
+      wBound_ = buffer_ + bufferSize_;

Review comment:
       Could you explain this change a bit? Originally the code always set `wBase_ = buffer_;` but after your change, `wBase_` is only set if the buffer is owned. Is that guaranteed correct?




----------------------------------------------------------------
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] [thrift] cfriedt commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-646578961


   AppVeyor is failing for completely unrelated reasons ("cannot used reserved language keyword return").
   
   Not even sure where to begin with Travis.
   
   Re-running tests locally on aarch64 and amd64.


----------------------------------------------------------------
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] [thrift] cfriedt edited a comment on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt edited a comment on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-674168678


   @Jens-G - will this be merged soon? I'm curious about the unrelated tests failing. Is there a stable commit that I could rebase from?
   
   I'd be happy to make additional changes if required.


----------------------------------------------------------------
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] [thrift] cfriedt edited a comment on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
cfriedt edited a comment on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-646578961


   AppVeyor is failing for completely unrelated reasons ("cannot used reserved language keyword return").
   
   Not even sure where to begin with Travis - failures seem to be mainly java related.
   
   Re-running tests locally on aarch64 and amd64.


----------------------------------------------------------------
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] [thrift] emmenlau commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
emmenlau commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-854452584


   @cfriedt I'll pull this into my local build and see how it performs, ok? Its not a merge yet, but at least a step on the way. Thanks again!


-- 
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] [thrift] emmenlau commented on pull request #2007: THRIFT-5093: TMemoryBuffer fix wpos for policy == OBSERVER

Posted by GitBox <gi...@apache.org>.
emmenlau commented on pull request #2007:
URL: https://github.com/apache/thrift/pull/2007#issuecomment-987823916


   > Man - I wish there were more hours in the day!
   
   Haha, don't we all! Happy coding and keep up the good work! Would be great if this comes to a good solution...


-- 
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: notifications-unsubscribe@thrift.apache.org

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