[ 
https://issues.apache.org/jira/browse/DISPATCH-1940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17386482#comment-17386482
 ] 

ASF GitHub Bot commented on DISPATCH-1940:
------------------------------------------

ganeshmurthy commented on a change in pull request #1301:
URL: https://github.com/apache/qpid-dispatch/pull/1301#discussion_r675799980



##########
File path: src/adaptors/http2/http2_adaptor.c
##########
@@ -770,63 +856,102 @@ static bool compose_and_deliver(qdr_http2_connection_t 
*conn, qdr_http2_stream_d
     if (!stream_data->header_and_props_composed) {
         qd_composed_field_t  *header_and_props = 0;
         if (conn->ingress) {
-            header_and_props = qd_message_compose_amqp(stream_data->message,
-                                                  conn->config->address,  // 
const char *to
-                                                  0,                      // 
const char *subject
-                                                  stream_data->reply_to,  // 
const char *reply_to
-                                                  0,                      // 
const char *content_type
-                                                  0,                      // 
const char *content_encoding
-                                                  0,                      // 
int32_t  correlation_id
-                                                  conn->config->site);
+            header_and_props = qd_message_compose_amqp(conn,
+                       stream_data->message,
+                                       conn->config->address,  // const char 
*to
+                                       0,                      // const char 
*subject
+                                       stream_data->reply_to,  // const char 
*reply_to
+                                       0,                      // const char 
*content_type
+                                       0,                      // const char 
*content_encoding
+                                       0,                      // int32_t  
correlation_id
+                                       conn->config->site);
         }
         else {
-            header_and_props = qd_message_compose_amqp(stream_data->message,
-                                                  stream_data->reply_to,  // 
const char *to
-                                                  0,                      // 
const char *subject
-                                                  0,                      // 
const char *reply_to
-                                                  0,                      // 
const char *content_type
-                                                  0,                      // 
const char *content_encoding
-                                                  0,                      // 
int32_t  correlation_id
-                                                  conn->config->site);
+            header_and_props = qd_message_compose_amqp(conn,
+                       stream_data->message,
+                       stream_data->reply_to,  // const char *to
+                                       0,                      // const char 
*subject
+                                       0,                      // const char 
*reply_to
+                                       0,                      // const char 
*content_type
+                                       0,                      // const char 
*content_encoding
+                                       0,                      // int32_t  
correlation_id
+                                       conn->config->site);
         }
 
         if (receive_complete) {
             qd_log(http2_adaptor->log_source, QD_LOG_TRACE, 
"[C%"PRIu64"][S%"PRId32"][L%"PRIu64"] receive_complete = true in 
compose_and_deliver", conn->conn_id, stream_data->stream_id, 
stream_data->in_link->identity);
-            if (!stream_data->body) {
-                stream_data->body = qd_compose(QD_PERFORMATIVE_BODY_DATA, 0);
-                qd_compose_insert_binary(stream_data->body, 0, 0);
-                qd_log(http2_adaptor->log_source, QD_LOG_TRACE, 
"[C%"PRIu64"][S%"PRId32"] Inserting empty body data in compose_and_deliver", 
conn->conn_id, stream_data->stream_id);
-            }
-
+            bool q2_blocked;
             if (stream_data->footer_properties) {
-                qd_message_compose_5(stream_data->message, header_and_props, 
stream_data->app_properties, stream_data->body, stream_data->footer_properties, 
receive_complete);
+                qd_message_compose_3(stream_data->message, header_and_props, 
stream_data->app_properties, receive_complete);
+                qd_message_stream_data_append(stream_data->message, 
&stream_data->body_buffers, &q2_blocked);
+                stream_data->body_data_added_to_msg = true;
+
+                qd_buffer_list_t existing_buffers;
+                DEQ_INIT(existing_buffers);
+                qd_compose_take_buffers(stream_data->footer_properties, 
&existing_buffers);
+                qd_message_stream_data_footer_append(stream_data->message, 
&existing_buffers);
             }
             else {
-                qd_message_compose_4(stream_data->message, header_and_props, 
stream_data->app_properties, stream_data->body, receive_complete);
+                qd_message_compose_3(stream_data->message, header_and_props, 
stream_data->app_properties, receive_complete);
+                qd_message_stream_data_append(stream_data->message, 
&stream_data->body_buffers, &q2_blocked);
+                stream_data->body_data_added_to_msg = true;
             }
+
+            conn->q2_blocked = conn->q2_blocked || q2_blocked;
+               if (conn->q2_blocked) {
+                       qd_log(http2_adaptor->protocol_log_source, 
QD_LOG_TRACE, "[C%"PRIu64"] q2 is blocked on this connection", conn->conn_id);
+               }
         }
         else {
-            if (stream_data->body) {
-                qd_log(http2_adaptor->log_source, QD_LOG_TRACE, 
"[C%"PRIu64"][S%"PRId32"][L%"PRIu64"] receive_complete = false and has 
stream_data->body in compose_and_deliver", conn->conn_id, 
stream_data->stream_id, stream_data->in_link->identity);
+            if (DEQ_SIZE(stream_data->body_buffers) > 0) {
+                qd_log(http2_adaptor->log_source, QD_LOG_TRACE, 
"[C%"PRIu64"][S%"PRId32"][L%"PRIu64"] receive_complete = false and has 
stream_data->body_buffers in compose_and_deliver", conn->conn_id, 
stream_data->stream_id, stream_data->in_link->identity);
+                bool q2_blocked;
                 if (stream_data->footer_properties) {
-                    qd_message_compose_5(stream_data->message, 
header_and_props, stream_data->app_properties, stream_data->body, 
stream_data->footer_properties, receive_complete);
+                       if (!stream_data->entire_footer_arrived) {
+                               qd_compose_free(header_and_props);
+                               return false;
+                       }
+
+                    qd_message_compose_3(stream_data->message, 
header_and_props, stream_data->app_properties, receive_complete);
+                    qd_message_stream_data_append(stream_data->message, 
&stream_data->body_buffers, &q2_blocked);
+                    qd_buffer_list_t existing_buffers;
+                    DEQ_INIT(existing_buffers);
+                    qd_compose_take_buffers(stream_data->footer_properties, 
&existing_buffers);
+                    qd_message_stream_data_footer_append(stream_data->message, 
&existing_buffers);
                 }
                 else {
-                    qd_message_compose_4(stream_data->message, 
header_and_props, stream_data->app_properties, stream_data->body, 
receive_complete);
+                       qd_message_compose_3(stream_data->message, 
header_and_props, stream_data->app_properties, receive_complete);
+                       qd_message_stream_data_append(stream_data->message, 
&stream_data->body_buffers, &q2_blocked);
                 }
-                stream_data->body_data_added = true;
+                stream_data->body_data_added_to_msg = true;
+                conn->q2_blocked = conn->q2_blocked || q2_blocked;
+                       if (conn->q2_blocked) {
+                               qd_log(http2_adaptor->protocol_log_source, 
QD_LOG_TRACE, "[C%"PRIu64"] q2 is blocked on this connection", conn->conn_id);
+                       }
             }
             else {
-
                 if (stream_data->footer_properties) {
+
+                       if (!stream_data->entire_footer_arrived) {
+                               qd_compose_free(header_and_props);
+                               return false;
+                       }
+
                     //
                     // The footer has already arrived but there was no body. 
Insert an empty body
                     //
-                    stream_data->body = qd_compose(QD_PERFORMATIVE_BODY_DATA, 
0);
-                    qd_message_compose_5(stream_data->message, 
header_and_props, stream_data->app_properties, stream_data->body, 
stream_data->footer_properties, receive_complete);
+                    qd_message_compose_3(stream_data->message, 
header_and_props, stream_data->app_properties, receive_complete);
+                    qd_message_stream_data_append(stream_data->message, 
&stream_data->body_buffers, 0);

Review comment:
       I will add the assert, thanks
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


> [http2] Router HTTP2 adaptor should pass h2spec
> -----------------------------------------------
>
>                 Key: DISPATCH-1940
>                 URL: https://issues.apache.org/jira/browse/DISPATCH-1940
>             Project: Qpid Dispatch
>          Issue Type: Bug
>          Components: Protocol Adaptors
>    Affects Versions: 1.15.0
>            Reporter: Jiri Daněk
>            Assignee: Ganesh Murthy
>            Priority: Major
>              Labels: crash
>         Attachments: h2spec.conf
>
>
> h2spec (https://github.com/summerwind/h2spec) is a HTTP/2 conformance 
> checker, mentioned in 
> https://blog.cloudflare.com/tools-for-debugging-testing-and-using-http-2
> Running h2spec against nghttpd directly produces only one failure
> {noformat}
> Failures: 
> Hypertext Transfer Protocol Version 2 (HTTP/2)
>   5. Streams and Multiplexing
>     5.1. Stream States
>       5.1.1. Stream Identifiers
>         using source address 127.0.0.1:53110
>         × 2: Sends stream identifier that is numerically smaller than previous
>           -> The endpoint MUST respond with a connection error of type 
> PROTOCOL_ERROR.
>              Expected: GOAWAY Frame (Error Code: PROTOCOL_ERROR)
>                        Connection closed
>                Actual: DATA Frame (length:147, flags:0x01, stream_id:5)
> {noformat}
> When Dispatch is put in between, it fails a few first checks, then dispatch 
> crashes
> {code}
> $ wget 
> https://github.com/summerwind/h2spec/releases/download/v2.6.0/h2spec_linux_amd64.tar.gz
> $ nghttpd --no-tls -D -d /tmp 8888
> $ qdrouterd -c h2spec.conf
> $ ./h2spec -p 24162
> {code}
> I get the following result every time I run the above commands
> {noformat}
>   2. Streams and Multiplexing
>     ✔ 1: Sends a PRIORITY frame on idle stream
>     using source address 127.0.0.1:35814half-closed (remote) stream
>     × 2: Sends a WINDOW_UPDATE frame on half-closed (remote) stream
>       -> The endpoint MUST accept WINDOW_UPDATE frame.
>          Expected: DATA frame
>            Actual: HEADERS Frame (length:69, flags:0x04, stream_id:1)
>     using source address 127.0.0.1:35816closed (remote) stream
>     × 3: Sends a PRIORITY frame on half-closed (remote) stream
>       -> The endpoint MUST accept PRIORITY frame.
>          Expected: DATA frame
>            Actual: HEADERS Frame (length:69, flags:0x04, stream_id:1)
>     ✔ 4: Sends a RST_STREAM frame on half-closed (remote) stream
>     ✔ 5: Sends a PRIORITY frame on closed stream
> {noformat}
> {noformat}
>     3.8. GOAWAY
>       using source address 127.0.0.1:35850
>       × 1: Sends a GOAWAY frame
>         -> The endpoint MUST accept GOAWAY frame.
>            Expected: Connection closed
>                      PING Frame (length:8, flags:0x01, stream_id:0, 
> opaque_data:h2spec)
>              Actual: Timeout
> {noformat}
> {noformat}
>   4. HTTP Message Exchanges
>     ✔ 1: Sends a GET request
>     ✔ 2: Sends a HEAD request
>     ✔ 3: Sends a POST request
>     using source address 127.0.0.1:35866ers
>     × 4: Sends a POST request with trailers
>       -> The endpoint MUST respond to the request.
>          Expected: HEADERS Frame (stream_id:1)
>            Actual: Connection closed
> {noformat}
> {noformat}
>   5. HPACK
>     × 1: Sends a indexed header field representation
> 2021-01-31 13:26:25.042679 +0100 ROUTER_CORE (trace) Core action 
> 'link_first_attach' (../src/router_core/router_core_thread.c:238)
> 2021-01-31 13:26:25.042912 +0100 ROUTER_CORE (info) [C31][L95] Link attached: 
> dir=out source={(dyn)<none> expire:link} target={<none> expire:link} 
> (../src/router_core/connections.c:1812)
> 2021-01-31 13:26:25.042983 +0100 HTTP_ADAPTOR (trace) [C31] Activation 
> triggered, calling pn_raw_connection_wake() 
> (../src/adaptors/http2/http2_adaptor.c:1524)
> ../src/router_core/delivery.c:109:19: runtime error: member access within 
> null pointer of type 'struct qdr_delivery_t'
> AddressSanitizer:DEADLYSIGNAL
> =================================================================
> ==567==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000018 (pc 
> 0x7fb3080acb9b bp 0x7fff063b3ea0 sp 0x7fff063b3e90 T0)
> ==567==The signal is caused by a WRITE memory access.
> ==567==Hint: address points to the zero page.
>     #0 0x7fb3080acb9b in sys_atomic_add ../include/qpid/dispatch/atomic.h:80
>     #1 0x7fb3080acc86 in sys_atomic_inc ../include/qpid/dispatch/atomic.h:209
>     #2 0x7fb3080ad65f in qdr_delivery_incref ../src/router_core/delivery.c:109
>     #3 0x7fb3080aeaa4 in qdr_delivery_continue 
> ../src/router_core/delivery.c:220
>     #4 0x7fb3081e69e2 in on_frame_recv_callback 
> ../src/adaptors/http2/http2_adaptor.c:988
>     #5 0x7fb307535d4d in nghttp2_session_mem_recv 
> (/nix/store/1blnfglp53fsrd8rjmrcql18k9hdimr7-nghttp2-1.41.0-lib/lib/libnghttp2.so.14+0x10d4d)
>     #6 0x7fb308200572 in handle_incoming_http 
> ../src/adaptors/http2/http2_adaptor.c:1925
>     #7 0x7fb30820998e in handle_connection_event 
> ../src/adaptors/http2/http2_adaptor.c:2297
>     #8 0x7fb3081ace0c in handle_event_with_context ../src/server.c:804
>     #9 0x7fb3081ace4d in do_handle_raw_connection_event ../src/server.c:810
>     #10 0x7fb3081b169c in handle ../src/server.c:1090
>     #11 0x7fb3081b195d in thread_run ../src/server.c:1122
>     #12 0x7fb3081b8c45 in qd_server_run ../src/server.c:1484
>     #13 0x4026e4 in main_process ../router/src/main.c:113
>     #14 0x404564 in main ../router/src/main.c:367
>     #15 0x7fb306a1dc7c in __libc_start_main 
> (/nix/store/9df65igwjmf2wbw0gbrrgair6piqjgmi-glibc-2.31/lib/libc.so.6+0x23c7c)
>     #16 0x402419 in _start 
> (/home/jdanek/repos/qpid/qpid-dispatch/cmake-build-debug/router/qdrouterd+0x402419)
> AddressSanitizer can not provide additional info.
> SUMMARY: AddressSanitizer: SEGV ../include/qpid/dispatch/atomic.h:80 in 
> sys_atomic_add
> ==567==ABORTING
> {noformat}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to