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.

Sign up for GitHub

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

Set BridgeResourceFolder at Bridge startup #337

Merged
merged 2 commits into from
Sep 16, 2015
Merged

Set BridgeResourceFolder at Bridge startup #337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cbd1bef
Set BridgeResourceFolder at Bridge startup
roncain Sep 15, 2015
26c08b1
Improved Bridge.exe console output to aid debugging
roncain Sep 16, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 4 additions & 34 deletions src/System.Private.ServiceModel/tests/Common/Infrastructure/src/BridgeClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,6 @@ private static void MakeConfigRequest()

private static string CreateConfigRequestContentAsJson()
{
ValidateConfigRequestProperties();

// Create a Json dictionary of name/value pairs from TestProperties
StringBuilder sb = new StringBuilder("{ ");
string[] propertyNames = TestProperties.PropertyNames.ToArray();
Expand All @@ -185,11 +183,10 @@ private static string CreateConfigRequestContentAsJson()
string propertyName = propertyNames[i];
string propertyValue = TestProperties.GetProperty(propertyName);

// If the Bridge is remote but the resources folder is local, omit it from the config request.
// In remote scenarios, either the Bridge must be started with its own resources folder or
// it is placed on a file share that we can reference from this application.
if ((String.Equals(propertyName, TestProperties.BridgeResourceFolder_PropertyName)) &&
(!IsBridgeHostedLocally && !IsPathRemote(propertyValue)))
// We do not configure the BridgeResourceFolder because the Bridge
// is provided one at startup. Moreover, cross-platform scenarios
// would require this folder to be shared across OS boundaries.
if (String.Equals(propertyName, TestProperties.BridgeResourceFolder_PropertyName, StringComparison.OrdinalIgnoreCase))
{
continue;
}
Expand All @@ -204,33 +201,6 @@ private static string CreateConfigRequestContentAsJson()
return sb.ToString();
}

// Validates some of the name/value pairs that will be sent to the Bridge
// via the /config POST request.
private static void ValidateConfigRequestProperties()
{
string bridgeResourceFolder = TestProperties.GetProperty(TestProperties.BridgeResourceFolder_PropertyName);

// Validate the Bridge resource folder exists (even if UNC).
if (String.IsNullOrEmpty(bridgeResourceFolder) || !Directory.Exists(bridgeResourceFolder))
{
throw new Exception(String.Format("BridgeResourceFolder '{0}' does not exist", bridgeResourceFolder));
}
}

// Returns true if the given file path is remote.
internal static bool IsPathRemote(string path)
{
if (String.IsNullOrEmpty(path))
{
return false;
}
if (new Uri(path).IsUnc)
{
return true;
}
return false;
}

private static string MakeResourcePutRequest(string resourceName)
{
EnsureBridgeIsRunning();
Expand Down
2 changes: 1 addition & 1 deletion src/System.Private.ServiceModel/tests/Common/Infrastructure/src/testproperties.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
</TestProperty>
<TestProperty Include="BridgeMaxIdleTimeSpan">
<Value>$(BridgeMaxIdleTimeSpan)</Value>
<DefaultValue>20:00</DefaultValue>
<DefaultValue>24:00:00</DefaultValue>
</TestProperty>
<TestProperty Include="MaxTestTimeSpan">
<Value>$(MaxTestTimeSpan)</Value>
Expand Down
10 changes: 8 additions & 2 deletions src/System.Private.ServiceModel/tools/setupfiles/CleanupWCFTestService.cmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,17 @@ setlocal

echo BridgeKeepRunning=%BridgeKeepRunning%
if '%BridgeKeepRunning%' neq 'true' (
echo Stopping the Bridge...
echo Releasing Bridge resources and stopping it if running locally.
pushd %~dp0..\..\..\..\bin\wcf\tools\Bridge
call Bridge.exe -stopIfLocal %*
echo Invoking Bridge.exe -stopIfLocal -reset %* ...
call Bridge.exe -stopIfLocal -reset %*
popd
) else (
echo Releasing Bridge resources but leaving it running.
pushd %~dp0..\..\..\..\bin\wcf\tools\Bridge
echo Invoking Bridge.exe -reset %* ...
call Bridge.exe -reset %*
popd
echo The Bridge was left running because BridgeKeepRunning is true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, CleanupWcfTestService.cmd is called immediately after a run happens. Do we necessarily want to get rid of all resources that the bridge is holding?

My flow of work is typically the following:

  • run startBridge.cmd
  • run build.cmd to look at what's wrong
  • make changes
  • rebuild.

Sometimes, in the "rebuild" step I run build.cmd, other times I build the project and test directly.
My suspicion is that if someone has left the bridge running, we should probably leave any resources opened alone, and rely on the developer to either exit the Bridge or call reset manually

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CleanupWcfTestService.cmd is the script called to clean up all resources -- that's its purpose. In this case, cleanup means remove all certs and firewall rules created, shutdown the AppDomains, and continue running. The Bridge will reacquire resources the next time it receives requests.

I think it is fairly critical that after a run all DLL's loaded from the BridgeResourceFolder be released, otherwise the build will fail because it cannot write the new DLL's into the BridgeResourceFolder. I think asking the developer to do an explicit 'reset' is just going to lead to confusing build failures.

)

Expand Down
4 changes: 2 additions & 2 deletions src/System.Private.ServiceModel/tools/setupfiles/SetupWCFTestService.cmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ echo Building the Bridge...
call BuildWCFTestService.cmd
popd

echo Starting the Bridge with parameters %*
pushd %~dp0..\..\..\..\bin\wcf\tools\Bridge
start /MIN Bridge.exe %*
echo starting the Bridge with arguments: /BridgeResourceFolder:..\..\Bridge\Resources %*
start /MIN Bridge.exe /BridgeResourceFolder:..\..\Bridge\Resources %*
popd

exit /b
Expand Down
2 changes: 1 addition & 1 deletion ...rviceModel/tools/test/Bridge.Build.Tasks/Bridge.Build.Tasks/ReleaseBridgeResourcesTask.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public override bool Execute()
bridgeIsRunning = true;

// A DELETE request to the config endpoint releases all allocated resources.
response = httpClient.DeleteAsync(bridgeAddress + "/config/").GetAwaiter().GetResult();
response = httpClient.DeleteAsync(bridgeAddress + "/resource/").GetAwaiter().GetResult();
if (!response.IsSuccessStatusCode)
{
string reason = String.Format("Failed to release the resources. The DELETE request to Bridge returned unexpected status code='{0}', reason='{1}'",
Expand Down
19 changes: 19 additions & 0 deletions src/System.Private.ServiceModel/tools/test/Bridge/Bridge/AppDomainManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ public static string OnResourceFolderChanged(string oldFolder, string newFolder)
[MethodImpl(MethodImplOptions.Synchronized)]
public static string CreateAppDomain(string path)
{
if (String.IsNullOrWhiteSpace(path))
{
throw new ArgumentNullException("path");
}

string friendlyName = "BridgeAppDomain" + TypeCache.AppDomains.Count;
var appDomainSetup = new AppDomainSetup();
appDomainSetup.ApplicationBase = path;
Expand All @@ -57,6 +62,9 @@ public static string CreateAppDomain(string path)
TypeCache.AppDomains.Add(friendlyName, newAppDomain);
TypeCache.Cache.Add(friendlyName, loader.GetTypes());

Trace.WriteLine(String.Format("{0:T} - Created new AppDomain '{1}'", DateTime.Now, friendlyName),
typeof(AppDomainManager).Name);

return friendlyName;
}

Expand All @@ -66,6 +74,8 @@ public static void ShutdownAllAppDomains()
{
ShutdownAppDomain(domainName);
}

ConfigController.CurrentAppDomainName = null;
}

public static void ShutdownAppDomain(string appDomainName)
Expand All @@ -85,6 +95,15 @@ public static void ShutdownAppDomain(string appDomainName)
AppDomain.Unload(appDomain);
TypeCache.AppDomains.Remove(appDomainName);
TypeCache.Cache.Remove(appDomainName);
Trace.WriteLine(String.Format("{0:T} - Shutdown AppDomain '{1}'", DateTime.Now, appDomainName),
typeof(AppDomainManager).Name);
}

// If this is the main AppDomain known to hold resources,
// reset to null to avoid further use.
if (String.Equals(appDomainName, ConfigController.CurrentAppDomainName, StringComparison.OrdinalIgnoreCase))
{
ConfigController.CurrentAppDomainName = null;
}
}
}
Expand Down
15 changes: 13 additions & 2 deletions src/System.Private.ServiceModel/tools/test/Bridge/Bridge/BridgeController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ public HttpResponseMessage Delete(HttpRequestMessage request)
}
}

public static void ReleaseAllResources()
// Release all Bridge resources.
// If 'force' is true, it means the Bridge is shutting down
// and even the firewall rules necessary to talk to the Bridge
// will be removed.
public static void ReleaseAllResources(bool force)
{
// Cleanly shutdown all AppDomains we own so they have
// the chance to release resources they've acquired or installed
Expand All @@ -99,11 +103,18 @@ public static void ReleaseAllResources()

// Finally remove all firewall rules we added for the ports
PortManager.RemoveAllBridgeFirewallRules();

// If the Bridge is not being shutdown, open a port in the firewall to
// communicate with the Bridge itself.
if (!force)
{
PortManager.OpenPortInFirewall(ConfigController.BridgeConfiguration.BridgePort);
}
}

public static void StopBridgeProcess(int exitCode)
{
ReleaseAllResources();
ReleaseAllResources(force:true);
Environment.Exit(exitCode);
}

Expand Down
63 changes: 24 additions & 39 deletions src/System.Private.ServiceModel/tools/test/Bridge/Bridge/ConfigController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class ConfigController : ApiController
BridgeMaxIdleTimeSpan = IdleTimeoutHandler.Default_MaxIdleTimeSpan
};

internal static string CurrentAppDomainName;
internal static string s_currentAppDomainName;

static ConfigController()
{
Expand Down Expand Up @@ -52,6 +52,29 @@ public static BridgeConfiguration BridgeConfiguration
s_bridgeConfiguration = value;
}
}

// Gets/sets the friendly name of the AppDomain holding
// the allocated resources. Creates the AppDomain on first use.
public static string CurrentAppDomainName
{
get
{
lock (ConfigLock)
{
if (s_currentAppDomainName == null)
{
s_currentAppDomainName = AppDomainManager.CreateAppDomain(ConfigController.BridgeConfiguration.BridgeResourceFolder);
}

return s_currentAppDomainName;
}
}
set
{
s_currentAppDomainName = value;
}
}

public HttpResponseMessage POST(HttpRequestMessage request)
{
// A configuration change can have wide impact, so we don't allow concurrent use
Expand Down Expand Up @@ -140,44 +163,6 @@ public HttpResponseMessage Get(HttpRequestMessage request)
return response;
}

// The DELETE Http verb means release all resources allocated
// and return to an initial state
public HttpResponseMessage Delete(HttpRequestMessage request)
{
Trace.WriteLine(String.Format("{0:T} - received DELETE request", DateTime.Now),
typeof(ConfigController).Name);

// A configuration change can have wide impact, so we don't allow concurrent use
lock (ConfigController.ConfigLock)
{
try {
if (!String.IsNullOrEmpty(CurrentAppDomainName))
{
// Signal change of resource folder from prior value to null.
string oldResourceFolder = BridgeConfiguration.BridgeResourceFolder;
BridgeConfiguration.BridgeResourceFolder = null;
if (ResourceFolderChanged != null)
{
ResourceFolderChanged(this, new ChangedEventArgs<string>(oldResourceFolder, null));
}
}
HttpResponseMessage response = request.CreateResponse(HttpStatusCode.OK);
response.Content = new StringContent("\"Bridge configuration has been cleared.\"");
response.Content.Headers.ContentType = new MediaTypeHeaderValue(JsonSerializer.JsonMediaType);
return response;
}
catch (Exception ex)
{
var exceptionResponse = ex.Message;
Trace.WriteLine(String.Format("{0:T} - DELETE config exception:{1}{2}",
DateTime.Now, Environment.NewLine, ex),
typeof(ConfigController).Name);

return request.CreateResponse(HttpStatusCode.BadRequest, exceptionResponse);
}
}
}

private static string PrepareConfigResponse(IEnumerable<string> types)
{
return string.Format(@"{{
Expand Down
2 changes: 1 addition & 1 deletion src/System.Private.ServiceModel/tools/test/Bridge/Bridge/IdleTimeoutManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Bridge
{
public class IdleTimeoutHandler : DelegatingHandler
{
public static readonly TimeSpan Default_MaxIdleTimeSpan = TimeSpan.FromMinutes(30);
public static readonly TimeSpan Default_MaxIdleTimeSpan = TimeSpan.FromHours(24);
private IdleTimeoutManager _timeoutManager;

private IdleTimeoutHandler(TimeSpan idleTimeout)
Expand Down
Loading