Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash in session_after_frame_sent1 #1588

Closed
ganeshmurthy opened this issue Jun 8, 2021 · 3 comments
Closed

Crash in session_after_frame_sent1 #1588

ganeshmurthy opened this issue Jun 8, 2021 · 3 comments
Labels

Comments

@ganeshmurthy
Copy link

ganeshmurthy commented Jun 8, 2021

I periodically send PING frames to a http2 server to keep the connection open. The server in this case is nghttpd. I also send other traffic to the server like simple GET requests etc. The PING frame is sent only if there is no traffic on the connection. To send the PING frame I do

`

static void send_ping_frame(qdr_http2_connection_t *conn)
{
qdr_http2_session_data_t *session_data = conn->session_data;
if (session_data->session) {
int ret_val = nghttp2_submit_ping(session_data->session, NGHTTP2_FLAG_NONE, 0);
nghttp2_session_send(session_data->session);
}
}
`

Sometimes, I end up with this crash

`==144937==ERROR: AddressSanitizer: SEGV on unknown address 0x00000000000c (pc 0x7f9f380bf262 bp 0x61e000034880 sp 0x7f9f2508edd0 T2)
==144937==The signal is caused by a READ memory access.
==144937==Hint: address points to the zero page.

#0 0x7f9f380bf262 in session_after_frame_sent1 /home/gmurthy/opensource/nghttp2/lib/nghttp2_session.c:2538
#1 0x7f9f380c0701 in nghttp2_session_mem_send_internal /home/gmurthy/opensource/nghttp2/lib/nghttp2_session.c:3176
#2 0x7f9f380c1949 in nghttp2_session_send /home/gmurthy/opensource/nghttp2/lib/nghttp2_session.c:3254
#3 0x7f9f38d8e692 in send_ping_frame /home/gmurthy/opensource/qpid-dispatch/src/adaptors/http2/http2_adaptor.c:934
#4 0x7f9f38db3107 in egress_conn_ping_sender /home/gmurthy/opensource/qpid-dispatch/src/adaptors/http2/http2_adaptor.c:2353
#5 0x7f9f38d6aade in qd_timer_visit /home/gmurthy/opensource/qpid-dispatch/src/timer.c:317
#6 0x7f9f38d593ec in handle /home/gmurthy/opensource/qpid-dispatch/src/server.c:1006
#7 0x7f9f38d5b074 in thread_run /home/gmurthy/opensource/qpid-dispatch/src/server.c:1121
#8 0x7f9f38be02cd in _thread_init /home/gmurthy/opensource/qpid-dispatch/src/posix/threading.c:172
#9 0x7f9f384d63f8  (/lib64/libpthread.so.0+0x93f8)
#10 0x7f9f3767b4c2 in __clone (/lib64/libc.so.6+0x1014c2)`

The frame on nghttp2_session.c:2538 seems to be NULL.
I printed the frame from inside nghttp2_submit_ping() and that frame is being handled successfully in session_after_frame_sent1(), not sure where this NULL frame came from.
Any comments appreciated while I look more into this (code is from master branch)

@tatsuhiro-t
Copy link
Member

It looks like nghttp2 tried to send DATA frame when crash happened. Do you call send_ping_frame in separate thread, and also call nghttp2 API calls in another thread? nghttp2 API must be called in a single thread per session unless you do protect it with mutex.

@ganeshmurthy
Copy link
Author

ok so you are saying two threads cannot act on the same nghttp2 session unless under lock.
We have a connection object which contains a session_data object as seen here - https://github.com/apache/qpid-dispatch/blob/main/src/adaptors/http2/http2_adaptor.h#L134
The session_data object houses the nghttp2_session object here - https://github.com/apache/qpid-dispatch/blob/main/src/adaptors/http2/http2_adaptor.h#L64
We are using Proton's raw connection API for IO - https://qpid.apache.org/releases/qpid-proton-0.34.0/proton/c/api/raw__connection_8h.html - which guarantees that it provides an event batch all events in which pertain to a single connection per thread.
So, technically, nghttp2 API must be already most surely be called in a single thread per session, which means locking in not necessary

But, thanks for your insight, I will look more deeply keeping what you said in mind.

Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants