Using `pshazz use` causes tab-completion to hang

[tynril]

As of this commit, using pshazz use in a session that has already been initialized makes pressing Tab cause an infinite recursion during which PowerShell uses 100% CPU and becomes unresponsive.

This is because the call to pshazz use re-runs lib/completion.ps1, and makes global:PshazzTabExpansionBackup point to pshazz's overwrite defined below. Running pshazz use again in the same session errors out because global:PshazzTabExpansionBackup is already defined (to be honest, I'm not quite sure why it doesn't error out the first time).

This can be worked around by reverting lib/completion.ps1 to a prior version, but that obviously breaks other modules hooking into tab completion.

Environment:

• PowerShell Core 7.0.1 (via Scoop)
• pshazz 0.2020.05.16 (via Scoop)

Tagging @h404bi since the commit message of the problematic revision mentions that they'd like to refactor this functionality in the future. :)

[chawyehsu]

@tynril Thank you for reporting the issue. Please edit the local lib/completion.ps1 and try this revision to see if it can fix the issue.

diff --git a/lib/completion.ps1 b/lib/completion.ps1
index ac92828..502139f 100644
--- a/lib/completion.ps1
+++ b/lib/completion.ps1
@@ -2,9 +2,10 @@

 # Backup previous TabExpansion function
 if (Test-Path Function:\TabExpansion) {
-    Rename-Item Function:\TabExpansion global:PshazzTabExpansionBackup
+    if (!(Test-Path Function:\PshazzTabExpansionBackup)) {
+        Rename-Item Function:\TabExpansion global:PshazzTabExpansionBackup
+    }
 }

[tynril]

This patch does not fix the issue.

The reproduction steps I have for it are simply that in my PowerShell profile file, I had:

pshazz init
pshazz use <theme name>

Doing the same should allow you to reproduce the problem locally.

I have also done a local test where, in addition to your patch, I have added this line in the innermost if statement, right before the call to Rename-Item:

Write-Host ${Function:TabExpansion}.File

Surprisingly, this prints the path to pshazz's completion.ps1 file, even though the rename only takes place afterwards. With that in mind, the following patch fixes the issue:

diff --git a/lib/completion.ps1 b/lib/completion.ps1
index ac92828..6bb3288 100644
--- a/lib/completion.ps1
+++ b/lib/completion.ps1
@@ -2,7 +2,9 @@

 # Backup previous TabExpansion function
 if (Test-Path Function:\TabExpansion) {
-    Rename-Item Function:\TabExpansion global:PshazzTabExpansionBackup
+    if ($MyInvocation.MyCommand.Path -ne ${Function:TabExpansion}.File) {
+        Rename-Item Function:\TabExpansion global:PshazzTabExpansionBackup
+    }
 }

 # Override TabExpansion function

Thought if I'm being honest, I'm not sure why. It's strange to me that the global TabExpansion function somehow appears to be already patched even though global:PshazzTabExpansionBackup is yet undefined.

[tynril]

Ah, I think I've had an idea as to what is happening.

I do not have any module that messes with TabCompletion locally, besides pshazz. So I think that what happens is that Test-Path Function:\TabExpansion returns $false the first time completion.ps1 is executed, which then defines that function below in the same file.

And at the second execution of completion.ps1, the Test-Path condition returns $true (because pshazz has previously defined that function), which means that pshazz's previous definition is renamed toglobal:PshazzTabExpansionBackup, andglobal:TabExpansion` is defined again, now calling itself recursively.

I was able to confirm that by adding a Write-Host statement in a new else clause for the if statement that checks if TabExpansion is defined. It is indeed printed, once, and then the second execution takes the branch.

With that, I've opened #108 which should provide a fix to the issue.

[chawyehsu]

Thanks for your investigation, I've merged the fix. I'm planning to use Register-ArgumentCompleter to rewrite pshazz's functionality of tab-completion to avoid manipulating $global things, but refactoring could take much time, and I'll start when I have that time.