Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/6602/1/src/hnbgw_hnbap.c
File src/hnbgw_hnbap.c:

Line 430:                       talloc_free(name);
> At this point I ask myself: who is deallocating a rejected hnb_context? As 
Good question. It seems that the hnb context should live as long as the 
accepted connection stays open.

This case is a bit tricky because hnbgw_tx_hnb_register_rej(ctx) only puts the 
message on the connection's Tx queue.

Transmission of this message occurs asynchronously in osmo_stream_srv_write() 
via osmo_select_main(). We cannot free the hnb context before transmission has 
completed. 
However, osmo_stream_srv_write() does not invoke a callback when it is done 
writing and frees the message buffer, as far as I can see. So there is 
currently no way to close the connection and free the context at that point.

It looks like the osmo_stream_srv API expects the API consumer to detect a 
closed connection via a failed read() from the socket (as done by 
hnb_read_cb()). This means we would have to carry some state across read() 
calls which allows us to detect that the above rejection condition has occurred.
The close_cb() callback for the connection is an interesting hook at first 
sight, but it is only invoked when the API consumer calls 
osmo_stream_srv_destroy() so it is not suitable in our situation either.

The hnbgw_rx_hnb_deregister() case is easy because it occurs in the Rx path 
which does not have to deal with queuing and can just close the connection and 
free the context.

Hmm...


-- 
To view, visit https://gerrit.osmocom.org/6602
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iffd441eb2b6b75dfbe001b49b01bea015ca6e11c
Gerrit-PatchSet: 1
Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling <ssperl...@sysmocom.de>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Stefan Sperling <ssperl...@sysmocom.de>
Gerrit-HasComments: Yes

Reply via email to