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 2021/03/09 02:31:52 UTC
[GitHub] [trafficserver] maskit opened a new pull request #7584: Call constructors and destructors for H1/2 Session/Transaction via ClassAllocator
maskit opened a new pull request #7584:
URL: https://github.com/apache/trafficserver/pull/7584
#6241 enabled to call constructors which receive arguments, and destructors.
----------------------------------------------------------------
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] maskit commented on pull request #7584: Call constructors and destructors for H1/2 Session/Transaction via ClassAllocator
Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #7584:
URL: https://github.com/apache/trafficserver/pull/7584#issuecomment-846358627
@zwoop We need to backport #7600 to 9.1.x for ^^^ this crash.
--
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] shinrich commented on pull request #7584: Call constructors and destructors for H1/2 Session/Transaction via ClassAllocator
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7584:
URL: https://github.com/apache/trafficserver/pull/7584#issuecomment-833920499
Asking for 9.1 because along with PR #6241 makes bringing h2 to origin branch back to 9.1 easier.
--
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] maskit merged pull request #7584: Call constructors and destructors for H1/2 Session/Transaction via ClassAllocator
Posted by GitBox <gi...@apache.org>.
maskit merged pull request #7584:
URL: https://github.com/apache/trafficserver/pull/7584
----------------------------------------------------------------
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] maskit commented on a change in pull request #7584: Call constructors and destructors for H1/2 Session/Transaction via ClassAllocator
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7584:
URL: https://github.com/apache/trafficserver/pull/7584#discussion_r590824763
##########
File path: proxy/http2/Http2SessionAccept.cc
##########
@@ -54,9 +54,8 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
}
Http2ClientSession *new_session = THREAD_ALLOC_INIT(http2ClientSessionAllocator, this_ethread());
- new (new_session) Http2ClientSession();
- new_session->acl = std::move(session_acl);
- new_session->accept_options = &options;
+ new_session->acl = std::move(session_acl);
+ new_session->accept_options = &options;
Review comment:
clang-format added them to align with the previous line.
----------------------------------------------------------------
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 #7584: Call constructors and destructors for H1/2 Session/Transaction via ClassAllocator
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7584:
URL: https://github.com/apache/trafficserver/pull/7584#discussion_r590527249
##########
File path: proxy/http2/Http2SessionAccept.cc
##########
@@ -54,9 +54,8 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
}
Http2ClientSession *new_session = THREAD_ALLOC_INIT(http2ClientSessionAllocator, this_ethread());
- new (new_session) Http2ClientSession();
- new_session->acl = std::move(session_acl);
- new_session->accept_options = &options;
+ new_session->acl = std::move(session_acl);
+ new_session->accept_options = &options;
Review comment:
Why the extra spaces?
----------------------------------------------------------------
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] ezelkow1 commented on pull request #7584: Call constructors and destructors for H1/2 Session/Transaction via ClassAllocator
Posted by GitBox <gi...@apache.org>.
ezelkow1 commented on pull request #7584:
URL: https://github.com/apache/trafficserver/pull/7584#issuecomment-846227247
Im wondering if this may be causing crashes in some instances? I was trying to get our 9.1.x build up and running and while it was running autest here we had a lot of failures. Tracking those down they pointed to failures whenever H2 was used, installed that rpm on a test machine just using the base config and installing the autest certs and setting up 443. Ran ats via gdb and issued the same curl and would get a crash:
`#0 0x0000000000000000 in ??
#1 0x00000000006057a0 in ClassAllocator<Http2Stream, true>::destroy_if_enabled (this=0x8bbca0 <http2StreamAllocator>, ptr=0x7fffc4b6d3e0) at ../../include/tscore/Allocator.h:180
#2 Http2Stream::terminate_if_possible() () at Http2Stream.cc:486
#3 0x0000000000607fff in Http2Stream::main_event_handler(int, void*) () at Http2Stream.cc:215
#4 0x00000000007b48ed in Continuation::handleEvent (data=0xbc18c0, event=103, this=0x7fffc4b6d3e0) at I_Continuation.h:219
#5 Continuation::handleEvent (data=0xbc18c0, event=103, this=0x7fffc4b6d3e0) at I_Continuation.h:215
#6 EThread::process_event(Event*, int) () at UnixEThread.cc:164
#7 0x00000000007b4eee in EThread::process_queue (this=this@entry=0x1884950, NegativeQueue=NegativeQueue@entry=0x3a8fe30, ev_count=ev_count@entry=0x3a8fe2c, nq_count=nq_count@entry=0x3a8fe28) at UnixEThread.cc:199
#8 0x00000000007b5318 in EThread::execute_regular() () at UnixEThread.cc:259
#9 0x00000000007b618e in EThread::execute (this=0x1884950) at UnixEThread.cc:364
#10 EThread::execute (this=0x1884950) at UnixEThread.cc:342
#11 0x00000000007b3e8a in spawn_thread_internal (a=0x2e85f90) at Thread.cc:92
#12 0x00007ffff6deee65 in start_thread () from /lib64/libpthread.so.0
#13 0x00007ffff60f488d in clone () from /lib64/libc.so.6`
I would get a proper 404 response, via H2, but it looks like it was crashing in the cleanup after the session was done
I backed our build up to just before this commit (b24f62f699d7ea9b7f4a6cd6168c005511fae666) and did not see the issue
--
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] maskit commented on a change in pull request #7584: Call constructors and destructors for H1/2 Session/Transaction via ClassAllocator
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7584:
URL: https://github.com/apache/trafficserver/pull/7584#discussion_r591928784
##########
File path: proxy/http2/Http2SessionAccept.cc
##########
@@ -54,9 +54,8 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
}
Http2ClientSession *new_session = THREAD_ALLOC_INIT(http2ClientSessionAllocator, this_ethread());
- new (new_session) Http2ClientSession();
- new_session->acl = std::move(session_acl);
- new_session->accept_options = &options;
+ new_session->acl = std::move(session_acl);
+ new_session->accept_options = &options;
Review comment:
I mean line 56 by the previous line.
----------------------------------------------------------------
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] ezelkow1 edited a comment on pull request #7584: Call constructors and destructors for H1/2 Session/Transaction via ClassAllocator
Posted by GitBox <gi...@apache.org>.
ezelkow1 edited a comment on pull request #7584:
URL: https://github.com/apache/trafficserver/pull/7584#issuecomment-846227247
@maskit Im wondering if this may be causing crashes in some instances? I was trying to get our 9.1.x build up and running and while it was running autest here we had a lot of failures. Tracking those down they pointed to failures whenever H2 was used, installed that rpm on a test machine just using the base config and installing the autest certs and setting up 443. Ran ats via gdb and issued the same curl and would get a crash:
`#0 0x0000000000000000 in ??
#1 0x00000000006057a0 in ClassAllocator<Http2Stream, true>::destroy_if_enabled (this=0x8bbca0 <http2StreamAllocator>, ptr=0x7fffc4b6d3e0) at ../../include/tscore/Allocator.h:180
#2 Http2Stream::terminate_if_possible() () at Http2Stream.cc:486
#3 0x0000000000607fff in Http2Stream::main_event_handler(int, void*) () at Http2Stream.cc:215
#4 0x00000000007b48ed in Continuation::handleEvent (data=0xbc18c0, event=103, this=0x7fffc4b6d3e0) at I_Continuation.h:219
#5 Continuation::handleEvent (data=0xbc18c0, event=103, this=0x7fffc4b6d3e0) at I_Continuation.h:215
#6 EThread::process_event(Event*, int) () at UnixEThread.cc:164
#7 0x00000000007b4eee in EThread::process_queue (this=this@entry=0x1884950, NegativeQueue=NegativeQueue@entry=0x3a8fe30, ev_count=ev_count@entry=0x3a8fe2c, nq_count=nq_count@entry=0x3a8fe28) at UnixEThread.cc:199
#8 0x00000000007b5318 in EThread::execute_regular() () at UnixEThread.cc:259
#9 0x00000000007b618e in EThread::execute (this=0x1884950) at UnixEThread.cc:364
#10 EThread::execute (this=0x1884950) at UnixEThread.cc:342
#11 0x00000000007b3e8a in spawn_thread_internal (a=0x2e85f90) at Thread.cc:92
#12 0x00007ffff6deee65 in start_thread () from /lib64/libpthread.so.0
#13 0x00007ffff60f488d in clone () from /lib64/libc.so.6`
I would get a proper 404 response, via H2, but it looks like it was crashing in the cleanup after the session was done
I backed our build up to just before this commit (b24f62f699d7ea9b7f4a6cd6168c005511fae666) and did not see the issue
--
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 #7584: Call constructors and destructors for H1/2 Session/Transaction via ClassAllocator
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7584:
URL: https://github.com/apache/trafficserver/pull/7584#discussion_r591674710
##########
File path: proxy/http2/Http2SessionAccept.cc
##########
@@ -54,9 +54,8 @@ Http2SessionAccept::accept(NetVConnection *netvc, MIOBuffer *iobuf, IOBufferRead
}
Http2ClientSession *new_session = THREAD_ALLOC_INIT(http2ClientSessionAllocator, this_ethread());
- new (new_session) Http2ClientSession();
- new_session->acl = std::move(session_acl);
- new_session->accept_options = &options;
+ new_session->acl = std::move(session_acl);
+ new_session->accept_options = &options;
Review comment:
It seems like line 58 only needs one space before the = . Looks like a bug in clang-format then.
----------------------------------------------------------------
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