Cleanup port forwarding service

- Use a concurrent collection instead of manually locking
- Do not forward HTTPS port when it is not enabled
- Created multiple rules (HTTP/HTTPS) in parallel instead of in sync
This commit is contained in:
Mark Monteiro 2020-04-06 00:05:47 -04:00
parent c78413cf7c
commit 26afb42a72

View file

@ -1,6 +1,7 @@
#pragma warning disable CS1591
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Net;
using System.Text;
@ -26,8 +27,8 @@ namespace Emby.Server.Implementations.EntryPoints
private readonly IServerConfigurationManager _config;
private readonly IDeviceDiscovery _deviceDiscovery;
private readonly object _createdRulesLock = new object();
private List<IPEndPoint> _createdRules = new List<IPEndPoint>();
private readonly ConcurrentDictionary<IPEndPoint, byte> _createdRules = new ConcurrentDictionary<IPEndPoint, byte>();
private Timer _timer;
private string _lastConfigIdentifier;
@ -98,7 +99,7 @@ namespace Emby.Server.Implementations.EntryPoints
NatUtility.DeviceFound += OnNatUtilityDeviceFound;
NatUtility.StartDiscovery();
_timer = new Timer(ClearCreatedRules, null, TimeSpan.FromMinutes(10), TimeSpan.FromMinutes(10));
_timer = new Timer((_) => _createdRules.Clear(), null, TimeSpan.FromMinutes(10), TimeSpan.FromMinutes(10));
_deviceDiscovery.DeviceDiscovered += OnDeviceDiscoveryDeviceDiscovered;
@ -117,26 +118,16 @@ namespace Emby.Server.Implementations.EntryPoints
_deviceDiscovery.DeviceDiscovered -= OnDeviceDiscoveryDeviceDiscovered;
}
private void ClearCreatedRules(object state)
{
lock (_createdRulesLock)
{
_createdRules.Clear();
}
}
private void OnDeviceDiscoveryDeviceDiscovered(object sender, GenericEventArgs<UpnpDeviceInfo> e)
{
NatUtility.Search(e.Argument.LocalIpAddress, NatProtocol.Upnp);
}
private void OnNatUtilityDeviceFound(object sender, DeviceEventArgs e)
private async void OnNatUtilityDeviceFound(object sender, DeviceEventArgs e)
{
try
{
var device = e.Device;
CreateRules(device);
await CreateRules(e.Device).ConfigureAwait(false);
}
catch (Exception ex)
{
@ -144,7 +135,7 @@ namespace Emby.Server.Implementations.EntryPoints
}
}
private async void CreateRules(INatDevice device)
private Task CreateRules(INatDevice device)
{
if (_disposed)
{
@ -153,50 +144,46 @@ namespace Emby.Server.Implementations.EntryPoints
// On some systems the device discovered event seems to fire repeatedly
// This check will help ensure we're not trying to port map the same device over and over
var address = device.DeviceEndpoint;
lock (_createdRulesLock)
if (!_createdRules.TryAdd(device.DeviceEndpoint, 0))
{
if (!_createdRules.Contains(address))
{
_createdRules.Add(address);
}
else
{
return;
}
return Task.CompletedTask;
}
try
{
await CreatePortMap(device, _appHost.HttpPort, _config.Configuration.PublicPort).ConfigureAwait(false);
}
catch (Exception ex)
{
_logger.LogError(ex, "Error creating http port map");
return;
}
return Task.WhenAll(CreatePortMaps(device));
}
try
private IEnumerable<Task> CreatePortMaps(INatDevice device)
{
yield return CreatePortMap(device, _appHost.HttpPort, _config.Configuration.PublicPort);
if (_appHost.EnableHttps)
{
await CreatePortMap(device, _appHost.HttpsPort, _config.Configuration.PublicHttpsPort).ConfigureAwait(false);
}
catch (Exception ex)
{
_logger.LogError(ex, "Error creating https port map");
yield return CreatePortMap(device, _appHost.HttpsPort, _config.Configuration.PublicHttpsPort);
}
}
private Task<Mapping> CreatePortMap(INatDevice device, int privatePort, int publicPort)
private async Task CreatePortMap(INatDevice device, int privatePort, int publicPort)
{
_logger.LogDebug(
"Creating port map on local port {0} to public port {1} with device {2}",
"Creating port map on local port {LocalPort} to public port {PublicPort} with device {DeviceEndpoint}",
privatePort,
publicPort,
device.DeviceEndpoint);
return device.CreatePortMapAsync(
new Mapping(Protocol.Tcp, privatePort, publicPort, 0, _appHost.Name));
try
{
var mapping = new Mapping(Protocol.Tcp, privatePort, publicPort, 0, _appHost.Name);
await device.CreatePortMapAsync(mapping).ConfigureAwait(false);
}
catch (Exception ex)
{
_logger.LogError(
ex,
"Error creating port map on local port {LocalPort} to public port {PublicPort} with device {DeviceEndpoint}.",
privatePort,
publicPort,
device.DeviceEndpoint);
}
}
/// <inheritdoc />