mirror/transmission
1
0
Fork 0
mirror of https://github.com/transmission/transmission synced 2024-12-22 07:42:37 +00:00
Code Issues Releases Wiki Activity

feat: allow port forwarding state to recover from error (#6718)

Browse source 
* feat: allow upnp to recover from errors

* feat: allow natpmp to recover from errors

* chore: housekeeping

* code review: explicitly list all states to start discovering from

* fix: recover from failed UPnP discovery

* refactor: remove `UpnpState::Failed`
This commit is contained in:
Yat Ho 2024-05-26 06:08:16 +08:00 committed by GitHub
parent edddf9d80e
commit 17c6ec755c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 10 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
libtransmission/port-forwarding-natpmp.cc
View file

@ -140,16 +140,10 @@ tr_natpmp::PulseResult tr_natpmp::pulse(tr_port local_port, bool is_enabled)
} }
} }
if (state_ == State::Idle) if ((state_ == State::Idle || state_ == State::Err) &&
(is_mapped_ ? tr_time() >= renew_time_ : is_enabled && has_discovered_))
{ {
if (is_enabled && !is_mapped_ && has_discovered_) state_ = State::SendMap;
{
state_ = State::SendMap;
}
else if (is_mapped_ && tr_time() >= renew_time_)
{
state_ = State::SendMap;
}
} }
if (state_ == State::SendMap && canSendCommand()) if (state_ == State::SendMap && canSendCommand())

18
libtransmission/port-forwarding-upnp.cc
View file

@ -39,7 +39,6 @@ namespace
enum class UpnpState : uint8_t enum class UpnpState : uint8_t
{ {
Idle, Idle,
Failed,
WillDiscover, // next action is upnpDiscover() WillDiscover, // next action is upnpDiscover()
Discovering, // currently making blocking upnpDiscover() call in a worker thread Discovering, // currently making blocking upnpDiscover() call in a worker thread
WillMap, // next action is UPNP_AddPortMapping() WillMap, // next action is UPNP_AddPortMapping()
@ -58,9 +57,7 @@ struct tr_upnp
~tr_upnp() ~tr_upnp()
{ {
TR_ASSERT(!isMapped); TR_ASSERT(!isMapped);
TR_ASSERT( TR_ASSERT(state == UpnpState::Idle || state == UpnpState::WillDiscover || state == UpnpState::Discovering);
state == UpnpState::Idle || state == UpnpState::Failed || state == UpnpState::WillDiscover ||
state == UpnpState::Discovering);
FreeUPNPUrls(&urls); FreeUPNPUrls(&urls);
} }
@ -298,7 +295,7 @@ tr_port_forwarding_state tr_upnpPulse(
} }
else else
{ {
handle->state = UpnpState::Failed; handle->state = UpnpState::WillDiscover;
tr_logAddDebug(fmt::format("UPNP_GetValidIGD failed: {} ({})", tr_strerror(errno), errno)); tr_logAddDebug(fmt::format("UPNP_GetValidIGD failed: {} ({})", tr_strerror(errno), errno));
tr_logAddDebug("If your router supports UPnP, please make sure UPnP is enabled!"); tr_logAddDebug("If your router supports UPnP, please make sure UPnP is enabled!");
} }
@ -306,15 +303,15 @@ tr_port_forwarding_state tr_upnpPulse(
freeUPNPDevlist(devlist); freeUPNPDevlist(devlist);
} }
if ((handle->state == UpnpState::Idle) && (handle->isMapped) && if (handle->state == UpnpState::Idle && handle->isMapped &&
(!is_enabled || handle->advertised_port != advertised_port || handle->local_port != local_port)) (!is_enabled || handle->advertised_port != advertised_port || handle->local_port != local_port))
{ {
handle->state = UpnpState::WillUnmap; handle->state = UpnpState::WillUnmap;
} }
if (is_enabled && handle->isMapped && do_port_check && if (is_enabled && handle->isMapped && do_port_check &&
((get_specific_port_mapping_entry(handle, "TCP") != UPNPCOMMAND_SUCCESS) || (get_specific_port_mapping_entry(handle, "TCP") != UPNPCOMMAND_SUCCESS ||
(get_specific_port_mapping_entry(handle, "UDP") != UPNPCOMMAND_SUCCESS))) get_specific_port_mapping_entry(handle, "UDP") != UPNPCOMMAND_SUCCESS))
{ {
tr_logAddInfo(fmt::format( tr_logAddInfo(fmt::format(
_("Local port {local_port} is not forwarded to {advertised_port}"), _("Local port {local_port} is not forwarded to {advertised_port}"),
@ -339,7 +336,7 @@ tr_port_forwarding_state tr_upnpPulse(
handle->local_port = {}; handle->local_port = {};
} }
if ((handle->state == UpnpState::Idle) && is_enabled && !handle->isMapped) if (handle->state == UpnpState::Idle && is_enabled && !handle->isMapped)
{ {
handle->state = UpnpState::WillMap; handle->state = UpnpState::WillMap;
} }
@ -376,15 +373,14 @@ tr_port_forwarding_state tr_upnpPulse(
fmt::arg("advertised_port", advertised_port.host()))); fmt::arg("advertised_port", advertised_port.host())));
handle->advertised_port = advertised_port; handle->advertised_port = advertised_port;
handle->local_port = local_port; handle->local_port = local_port;
handle->state = UpnpState::Idle;
} }
else else
{ {
tr_logAddInfo(_("If your router supports UPnP, please make sure UPnP is enabled!")); tr_logAddInfo(_("If your router supports UPnP, please make sure UPnP is enabled!"));
handle->advertised_port = {}; handle->advertised_port = {};
handle->local_port = {}; handle->local_port = {};
handle->state = UpnpState::Failed;
} }
handle->state = UpnpState::Idle;
} }
return port_fwd_state(handle->state, handle->isMapped); return port_fwd_state(handle->state, handle->isMapped);