Skip to content

Commit b7e38ae

Browse files
ricardolleiteDuarte Fonseca
ricardolleite
authored and
Duarte Fonseca
committed
lse: fix missing timeout handling
Details This fixes the "3s delays". What happens goes more or less as follows, for an hypothetical Android application "AAP" Android suspends AAP -> VSIP connection times out, on VSIP due to missing timeout error handling, nothing happens Android resumes AAP reconnects to VSIP, re-establishes AAP -> VSIP, sends ASSIGN_CLIENT VSIP still has a record of the "old" connection, so it ignores the new one, and sends ASSIGN_CLIENT_ACK to the.. old connection (this causes a send_cbk error, visible in the logs) AAP times out after 3s, closes connection, goes to step 5 VSIP sees closed connection, cleans connection for client, which heals the situation for the next AAP attempt Besides the missing timeout handling, improve logging, and improve a bit of the client connection registration handling
1 parent c0173a7 commit b7e38ae

File tree

2 files changed

+63
-32
lines changed

2 files changed

+63
-32
lines changed

‎implementation/endpoints/src/local_tcp_server_endpoint_impl.cpp‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,8 @@ bool local_tcp_server_endpoint_impl::add_connection(const client_t &_client,
225225
connections_[_client] = _connection;
226226
ret = true;
227227
} else {
228-
VSOMEIP_WARNING << "Attempt to add already existing "
229-
"connection to client " << std::hex << _client << " endpoint > " << this;
228+
VSOMEIP_WARNING << "Attempt to add already existing connection to client " << std::hex
229+
<< _client << " endpoint > " << this;
230230
}
231231
return ret;
232232
}
@@ -235,7 +235,10 @@ void local_tcp_server_endpoint_impl::remove_connection(
235235
const client_t &_client) {
236236

237237
std::lock_guard<std::mutex> its_lock(connections_mutex_);
238-
connections_.erase(_client);
238+
if (!connections_.erase(_client)) {
239+
VSOMEIP_WARNING << "Client " << std::hex << _client << " has no registered connection to "
240+
<< " remove, endpoint > " << this;
241+
}
239242
}
240243

241244
void local_tcp_server_endpoint_impl::accept_cbk(
@@ -718,9 +721,20 @@ void local_tcp_server_endpoint_impl::connection::receive_cbk(
718721
client_t its_client = its_server->assign_client(
719722
&recv_buffer_[its_start], uint32_t(its_end - its_start));
720723
{
721-
set_bound_client(its_client);
724+
// order matters, register connection first, as it can fail
725+
if (!its_server->add_connection(its_client, shared_from_this())) {
726+
VSOMEIP_WARNING << std::hex << "Client 0x" << its_host->get_client()
727+
<< " is rejecting new connection with client ID 0x"
728+
<< its_client << " uid/gid= " << std::dec
729+
<< sec_client_.user << "/" << sec_client_.group
730+
<< " because of already existing connection using same "
731+
"client ID";
732+
733+
stop();
734+
return;
735+
}
722736
its_host->add_known_client(its_client, get_bound_client_host());
723-
its_server->add_connection(its_client, shared_from_this());
737+
set_bound_client(its_client);
724738
}
725739
its_server->send_client_identifier(its_client);
726740
assigned_client_ = true;
@@ -798,14 +812,23 @@ void local_tcp_server_endpoint_impl::connection::receive_cbk(
798812
} while (recv_buffer_size_ > 0 && found_message);
799813
}
800814

801-
if (is_stopped_ || _error == boost::asio::error::eof
802-
|| _error == boost::asio::error::connection_reset || is_error) {
815+
if (is_stopped_ || is_error || _error == boost::asio::error::eof
816+
|| _error == boost::asio::error::timed_out || _error == boost::asio::error::bad_descriptor
817+
|| _error == boost::asio::error::connection_reset) {
818+
VSOMEIP_INFO << "ltsei::receive_cbk closing connection due to is_stopped " << is_stopped_
819+
<< ", error '" << _error.message() << "', is_error " << is_error
820+
<< "', endpoint > " << this;
803821

804822
shutdown_and_close();
805823
its_server->remove_connection(bound_client_);
806824
its_server->configuration_->get_policy_manager()->remove_client_to_sec_client_mapping(bound_client_);
825+
} else {
826+
if (_error) {
827+
VSOMEIP_WARNING << "ltsei::receive_cbk received err '" << _error.message()
828+
<< "', endpoint > " << this;
829+
}
807830

808-
} else if (_error != boost::asio::error::bad_descriptor) {
831+
// schedule next read
809832
start();
810833
}
811834
}

‎implementation/endpoints/src/local_uds_server_endpoint_impl.cpp‎

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -750,21 +750,23 @@ void local_uds_server_endpoint_impl::connection::receive_cbk(
750750
client_t its_client = its_server->assign_client(
751751
&recv_buffer_[its_start], uint32_t(its_end - its_start));
752752

753-
if (its_config && its_config->is_security_enabled()) {
754-
// Add to known clients (loads new config if needed)
755-
its_host->add_known_client(its_client, get_bound_client_host());
753+
if (!its_server->add_connection(its_client, shared_from_this())) {
754+
VSOMEIP_WARNING
755+
<< std::hex << "Client 0x" << its_host->get_client()
756+
<< " is rejecting new connection with client ID 0x" << its_client
757+
<< " uid/gid= " << std::dec << sec_client_.user << "/"
758+
<< sec_client_.group
759+
<< " because of already existing connection using same client ID";
756760

757-
if (!its_server->add_connection(its_client, shared_from_this())) {
758-
VSOMEIP_WARNING << std::hex << "Client 0x" << its_host->get_client()
759-
<< " is rejecting new connection with client ID 0x" << its_client
760-
<< " uid/gid= " << std::dec
761-
<< sec_client_.user << "/"
762-
<< sec_client_.group
763-
<< " because of already existing connection using same client ID";
764-
stop();
765-
return;
766-
} else if (!its_server->configuration_->get_policy_manager()->check_credentials(
767-
its_client, &sec_client_)) {
761+
stop();
762+
return;
763+
}
764+
765+
its_host->add_known_client(its_client, get_bound_client_host());
766+
767+
if (its_config && its_config->is_security_enabled()) {
768+
if (!its_server->configuration_->get_policy_manager()->check_credentials(
769+
its_client, &sec_client_)) {
768770
VSOMEIP_WARNING << std::hex << "Client 0x" << its_host->get_client()
769771
<< " received client credentials from client 0x" << its_client
770772
<< " which violates the security policy : uid/gid="
@@ -776,14 +778,9 @@ void local_uds_server_endpoint_impl::connection::receive_cbk(
776778
stop();
777779
return;
778780
}
779-
else {
780-
set_bound_client(its_client);
781-
}
782-
} else {
783-
set_bound_client(its_client);
784-
its_host->add_known_client(its_client, get_bound_client_host());
785-
its_server->add_connection(its_client, shared_from_this());
786781
}
782+
783+
set_bound_client(its_client);
787784
its_server->send_client_identifier(its_client);
788785
assigned_client_ = true;
789786
} else if (!its_server->is_routing_endpoint_ || assigned_client_) {
@@ -835,12 +832,23 @@ void local_uds_server_endpoint_impl::connection::receive_cbk(
835832
} while (recv_buffer_size_ > 0 && found_message);
836833
}
837834

838-
if (is_stopped_ || _error == boost::asio::error::eof
839-
|| _error == boost::asio::error::connection_reset || is_error) {
835+
if (is_stopped_ || is_error || _error == boost::asio::error::eof
836+
|| _error == boost::asio::error::timed_out || _error == boost::asio::error::bad_descriptor
837+
|| _error == boost::asio::error::connection_reset) {
838+
VSOMEIP_INFO << "lusei::receive_cbk closing connection due to is_stopped " << is_stopped_
839+
<< ", error '" << _error.message() << "', is_error " << is_error
840+
<< ", endpoint > " << this;
841+
840842
shutdown_and_close();
841843
its_server->remove_connection(bound_client_);
842844
its_server->configuration_->get_policy_manager()->remove_client_to_sec_client_mapping(bound_client_);
843-
} else if (_error != boost::asio::error::bad_descriptor) {
845+
} else {
846+
if (_error) {
847+
VSOMEIP_WARNING << "lusei::receive_cbk received err '" << _error.message()
848+
<< "', endpoint > " << this;
849+
}
850+
851+
// schedule next read
844852
start();
845853
}
846854
}

0 commit comments

Comments
 (0)