Skip to content

Clean up code regarding AppDomain.CreateDomain and AppDomain.Unload #15554

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

Merged
rjmholt merged 2 commits into from
Jun 10, 2021
Merged

Clean up code regarding AppDomain.CreateDomain and AppDomain.Unload #15554

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
ef91ddd
Fix the nullref in parser
daxian-dbw Jun 9, 2021
e2dc401
Clean up code regarding `AppDomain.CreateDomain` and `AppDomain.Unload`
daxian-dbw Jun 10, 2021
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
116 changes: 52 additions & 64 deletions src/System.Management.Automation/engine/CommandCompletion/CommandCompletion.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,94 +526,82 @@ private static CommandCompletion CompleteInputImpl(Ast ast, Token[] tokens, IScr
{
var context = LocalPipeline.GetExecutionContextFromTLS();

bool cleanupModuleAnalysisAppDomain = context.TakeResponsibilityForModuleAnalysisAppDomain();
// First, check if a V1/V2 implementation of TabExpansion exists. If so, the user had overridden
// the built-in version, so we should continue to use theirs.
int replacementIndex = -1;
int replacementLength = -1;
List<CompletionResult> results = null;

try
if (NeedToInvokeLegacyTabExpansion(powershell))
{
// First, check if a V1/V2 implementation of TabExpansion exists. If so, the user had overridden
// the built-in version, so we should continue to use theirs.
int replacementIndex = -1;
int replacementLength = -1;
List<CompletionResult> results = null;
var inputAndCursor = GetInputAndCursorFromAst(positionOfCursor);
results = InvokeLegacyTabExpansion(powershell, inputAndCursor.Item1, inputAndCursor.Item2, false, out replacementIndex, out replacementLength);
replacementIndex += inputAndCursor.Item3;
}

if (NeedToInvokeLegacyTabExpansion(powershell))
if (results == null || results.Count == 0)
{
/* BROKEN code commented out, fix sometime
// If we were invoked from TabExpansion2, we want to "remove" TabExpansion2 and anything it calls
// from our results. We do this by faking out the session so that TabExpansion2 isn't anywhere to be found.
MutableTuple tupleForFrameToSkipPast = null;
foreach (var stackEntry in context.Debugger.GetCallStack())
{
var inputAndCursor = GetInputAndCursorFromAst(positionOfCursor);
results = InvokeLegacyTabExpansion(powershell, inputAndCursor.Item1, inputAndCursor.Item2, false, out replacementIndex, out replacementLength);
replacementIndex += inputAndCursor.Item3;
dynamic stackEntryAsPSObj = PSObject.AsPSObject(stackEntry);
if (stackEntryAsPSObj.Command.Equals("TabExpansion2", StringComparison.OrdinalIgnoreCase))
{
tupleForFrameToSkipPast = stackEntry.FunctionContext._localsTuple;
break;
}
}

if (results == null || results.Count == 0)
SessionStateScope scopeToRestore = null;
if (tupleForFrameToSkipPast != null)
{
/* BROKEN code commented out, fix sometime
// If we were invoked from TabExpansion2, we want to "remove" TabExpansion2 and anything it calls
// from our results. We do this by faking out the session so that TabExpansion2 isn't anywhere to be found.
MutableTuple tupleForFrameToSkipPast = null;
foreach (var stackEntry in context.Debugger.GetCallStack())
// Find this tuple in the scope stack.
scopeToRestore = context.EngineSessionState.CurrentScope;
var scope = context.EngineSessionState.CurrentScope;
while (scope != null && scope.LocalsTuple != tupleForFrameToSkipPast)
{
dynamic stackEntryAsPSObj = PSObject.AsPSObject(stackEntry);
if (stackEntryAsPSObj.Command.Equals("TabExpansion2", StringComparison.OrdinalIgnoreCase))
{
tupleForFrameToSkipPast = stackEntry.FunctionContext._localsTuple;
break;
}
scope = scope.Parent;
}

SessionStateScope scopeToRestore = null;
if (tupleForFrameToSkipPast != null)
if (scope != null)
{
// Find this tuple in the scope stack.
scopeToRestore = context.EngineSessionState.CurrentScope;
var scope = context.EngineSessionState.CurrentScope;
while (scope != null && scope.LocalsTuple != tupleForFrameToSkipPast)
{
scope = scope.Parent;
}

if (scope != null)
{
context.EngineSessionState.CurrentScope = scope.Parent;
}
context.EngineSessionState.CurrentScope = scope.Parent;
}
}

try
{
*/
var completionAnalysis = new CompletionAnalysis(ast, tokens, positionOfCursor, options);
results = completionAnalysis.GetResults(powershell, out replacementIndex, out replacementLength);
/*
}
finally
try
{
*/
var completionAnalysis = new CompletionAnalysis(ast, tokens, positionOfCursor, options);
results = completionAnalysis.GetResults(powershell, out replacementIndex, out replacementLength);
/*
}
finally
{
if (scopeToRestore != null)
{
if (scopeToRestore != null)
{
context.EngineSessionState.CurrentScope = scopeToRestore;
}
context.EngineSessionState.CurrentScope = scopeToRestore;
}
*/
}
*/
}

var completionResults = results ?? EmptyCompletionResult;
var completionResults = results ?? EmptyCompletionResult;

#if LEGACYTELEMETRY
// no telemetry here. We don't capture tab completion performance.
sw.Stop();
TelemetryAPI.ReportTabCompletionTelemetry(sw.ElapsedMilliseconds, completionResults.Count,
completionResults.Count > 0 ? completionResults[0].ResultType : CompletionResultType.Text);
#endif
return new CommandCompletion(
new Collection<CompletionResult>(completionResults),
-1,
replacementIndex,
replacementLength);
}
finally
{
if (cleanupModuleAnalysisAppDomain)
{
context.ReleaseResponsibilityForModuleAnalysisAppDomain();
}
}
return new CommandCompletion(
new Collection<CompletionResult>(completionResults),
-1,
replacementIndex,
replacementLength);
}
}

Expand Down
7 changes: 0 additions & 7 deletions src/System.Management.Automation/engine/CommandDiscovery.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1053,7 +1053,6 @@ private static CommandInfo TryModuleAutoDiscovery(string commandName,
if (etwEnabled) CommandDiscoveryEventSource.Log.ModuleAutoDiscoveryStart(commandName);

CommandInfo result = null;
bool cleanupModuleAnalysisAppDomain = false;
try
{
// If commandName had a slash, it was module-qualified or path-qualified.
Expand All @@ -1076,8 +1075,6 @@ private static CommandInfo TryModuleAutoDiscovery(string commandName,
discoveryTracer.WriteLine("Executing non module-qualified search: {0}", commandName);
context.CommandDiscovery.RegisterLookupCommandInfoAction("ActiveModuleSearch", commandName);

cleanupModuleAnalysisAppDomain = context.TakeResponsibilityForModuleAnalysisAppDomain();

// Get the available module files, preferring modules from $PSHOME so that user modules don't
// override system modules during auto-loading
if (etwEnabled) CommandDiscoveryEventSource.Log.SearchingForModuleFilesStart();
Expand Down Expand Up @@ -1142,10 +1139,6 @@ private static CommandInfo TryModuleAutoDiscovery(string commandName,
finally
{
context.CommandDiscovery.UnregisterLookupCommandInfoAction("ActiveModuleSearch", commandName);
if (cleanupModuleAnalysisAppDomain)
{
context.ReleaseResponsibilityForModuleAnalysisAppDomain();
}
}

if (etwEnabled) CommandDiscoveryEventSource.Log.ModuleAutoDiscoveryStop(commandName);
Expand Down
34 changes: 0 additions & 34 deletions src/System.Management.Automation/engine/ExecutionContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,40 +205,6 @@ internal bool ShouldTraceStatement
/// </summary>
internal string ModuleBeingProcessed { get; set; }

private bool _responsibilityForModuleAnalysisAppDomainOwned;

internal bool TakeResponsibilityForModuleAnalysisAppDomain()
{
if (_responsibilityForModuleAnalysisAppDomainOwned)
{
return false;
}

Diagnostics.Assert(AppDomainForModuleAnalysis == null, "Invalid module analysis app domain state");
_responsibilityForModuleAnalysisAppDomainOwned = true;
return true;
}

internal void ReleaseResponsibilityForModuleAnalysisAppDomain()
{
Diagnostics.Assert(_responsibilityForModuleAnalysisAppDomainOwned, "Invalid module analysis app domain state");

if (AppDomainForModuleAnalysis != null)
{
AppDomain.Unload(AppDomainForModuleAnalysis);
AppDomainForModuleAnalysis = null;
}

_responsibilityForModuleAnalysisAppDomainOwned = false;
}

/// <summary>
/// The AppDomain currently being used for module analysis. It should only be created if needed,
/// but various callers need to take responsibility for unloading the domain via
/// the TakeResponsibilityForModuleAnalysisAppDomain.
/// </summary>
internal AppDomain AppDomainForModuleAnalysis { get; set; }

/// <summary>
/// Authorization manager for this runspace.
/// </summary>
Expand Down
1 change: 0 additions & 1 deletion src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,6 @@ private void ImportModule_ViaAssembly(ImportModuleOptions importModuleOptions, A
if (!moduleLoaded)
{
PSModuleInfo module = LoadBinaryModule(
trySnapInName: false,
moduleName: null,
fileName: null,
suppliedAssembly,
Expand Down
Loading