Isn't this assignment superfluous?

I think it's more of a coding habit. I always make variable assigned before use.

:-) I always thought that Microsoft is strict in templates. It turns out that Microsoft indulge habits

@iSazonov Actually I suggest to always assign variables before use. I believe that will help prevent bugs. Sometimes C# compiler will also make you explicitly assign a variable before using it when it cannot figure out from the code path that it will be assigned for sure.

Thanks! It is good habit. :-)

I wonder, could it happen for any other reason.

I think this code is better, thank you for taking time to dig into it! 👍

This scenario involves switching SessionState within the same runspace, and it looks to me only will happen when a module is reloaded without changing content.

What happens if an instance of the class still exists? It would start executing against the new session state instead of the one in which is was created.

For me, at least, the main use-case for reloading of modules containing classes is to re-run unit tests against them after changes to the module/classes. Generally-speaking those unit tests re-create their objects anyway.

I'm not sure what should happen to existing objects when their originating class definitions are changed and reloaded, but I also haven't thought of a good reason to use objects across reloads so I don't expect to rely on the behavior - whatever it is.

I agree this is not a primary scenario, but it will happen (probably not intentionally) so we need to decide what should happen. For example, could it be a security issue to somehow get some instance to execute attackers code?

For existing class instances, I think they will continue to run against the original session state because the session state to be used is stored in _sessionStateField when an instance is constructed, see the code here.

Here is what I got in such scenario with the fix:
test.psm1

$passedArgs = $args
function Get-PassedValue { $passedArgs }
class Root
{
    $property = $passedArgs

    [object] GetPassedInValue()
    {
        return (Get-PassedValue)
    }
}
function Get-Root { [Root]::new() }

Run existing class instance after updating the SessionStateKeepper map

PS F:\tmp> Import-Module .\test.psm1 -ArgumentList 'value1'
PS F:\tmp> $root = Get-Root
PS F:\tmp> $root.property
value1
PS F:\tmp> $root.GetPassedInValue()
value1
PS F:\tmp> Import-Module .\test.psm1 -ArgumentList 'value2' -Force
PS F:\tmp>
PS F:\tmp> $soot = Get-Root
PS F:\tmp> $soot.property
value2
PS F:\tmp> $soot.GetPassedInValue()
value2
PS F:\tmp>
PS F:\tmp> $root.property
value1
PS F:\tmp> $root.GetPassedInValue()
value1
PS F:\tmp> 

You can see that $root is an existing class instance, and it still runs against the SessionState from the old module after Import-Module -Force, while $soot, a new instance, is running against the SessionState from the new module.

My question is what will happen if a reload module crash?

In that case, you won't get any new instances of the class, but existing class instances will continue to work since it's running against the old module instance.

It's good!

It will still be confusing with functions that accept parameters of module class type. The old instances will not bind and the error message is not helping to understand what is happening.

I think this is a fairly common use case, or at least we want it to be (i.e. interactive and iterative development of a module).

@powercode Do you mean continue to use old class instances after reloading the module? I don't think that's a common use case in module development. For most of time, use of old instances should not be intentional after the module gets reloaded.

It's sometimes handy to use New-Module instead of creating a file.

The dynamic modules created by New-Module behave differently from file-backed modules. For example, a dynamic modules' members are usable without importing the module despite that the module cannot be retrieved using Get-Module (unless you import it first). I'm not sure whether the differences are significant in this case, but it seems like to be safe file-backed modules should be used for testing.

I just tried using New-Module, and it doesn't work as expected -- Get-PassedArgsRoot doesn't return anything, and PSv5.1 has the same behavior. Maybe there is a bug somewhere ☹️

PS D:\> New-Module -Name ReloadingTest -ScriptBlock {
>> $passedArgs = $args
>> class Root { $passedIn = $passedArgs }
>> function Get-PassedArgsRoot { [Root]::new().passedIn }
>> function Get-PassedArgsNoRoot { $passedArgs }
>> } -ArgumentList 'abc'

ModuleType Version    Name                                ExportedCommands
---------- -------    ----                                ----------------
Script     0.0        ReloadingTest                       {Get-PassedArgsNoRoot, Get-PassedArgsRoot}

PS D:\>
PS D:\> Get-PassedArgsRoot
PS D:\>
PS D:\> Get-PassedArgsRoot
PS D:\>
PS D:\> Get-PassedArgsNoRoot
abc
PS D:\>

@daxian-dbw I couldn't reproduce your last post. I wrote another test that creates a dynamic module with arguments, then creates (overwrites?) another identical module with different arguments, then creates another with a slightly different class definition.

The results are the same with PowerShell 5.0, 5.1, and 6.0.0-alpha.13:

• fresh module works as expected
• creating new module that differs only by module arguments results in stale data returned by class instance
• creating new module whose scriptblock differs slightly works as expected

This behavior seems very similar to how I understand reloading of file-backed modules behaves.

Maybe there is a bug somewhere

It seems Yes.

>$a=New-Module -Name ReloadingTest -ScriptBlock {
  $passedArgs = $args
  class Root { $passedIn = $passedArgs }
  function Get-PassedArgsRoot { ([Root]::new()).passedIn }
  function Get-PassedArgsNoRoot { $passedArgs }
  } -ArgumentList 'abc'

> $a.GetExportedTypeDefinitions()

Key  Value
---  -----
Root class Root { $passedIn = $passedArgs }

>[root]
Unable to find type [root].
At line:1 char:1
+ [root]
+ ~~~~~~
    + CategoryInfo          : InvalidOperation: (root:TypeName) [], RuntimeException
    + FullyQualifiedErrorId : TypeNotFound

> class Root { $passedIn = $passedArgs }

> [root]

IsPublic IsSerial Name                                     BaseType
-------- -------- ----                                     --------
True     False    Root                                     System.Object

With regard to reload modules it seems there are differences between script, binary (assembly) and dynamic modules which require test them all.

@alx9r I can reproduce it in both PSv5.1 and PSv6.0.0-alpha.13. I opened the issue #2841 for this.

PSv5.1

PS:3> New-Module -Name ReloadingTest -ScriptBlock {
>>  $passedArgs = $args
>>  class Root { $passedIn = $passedArgs }
>>  function Get-PassedArgsRoot { [Root]::new().passedIn }
>>  function Get-PassedArgsNoRoot { $passedArgs }
>> } -ArgumentList 'abc'

ModuleType Version    Name                                ExportedCommands
---------- -------    ----                                ----------------
Script     0.0        ReloadingTest                       {Get-PassedArgsNoRoot, Get-PassedArgsRoot}


[C:\]
PS:4> Get-PassedArgsRoot
[C:\]
PS:5> Get-PassedArgsNoRoot
abc
[C:\]
PS:6> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.14393.206
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.14393.206
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

PSv6.0.0-alpha.13

PS C:\> New-Module -Name ReloadingTest -ScriptBlock {
>>  $passedArgs = $args
>>  class Root { $passedIn = $passedArgs }
>>  function Get-PassedArgsRoot { [Root]::new().passedIn }
>>  function Get-PassedArgsNoRoot { $passedArgs }
>> } -ArgumentList 'abc'

ModuleType Version    Name                                ExportedCommands
---------- -------    ----                                ----------------
Script     0.0        ReloadingTest                       {Get-PassedArgsNoRoot, Get-PassedArgsRoot}


PS C:\>
PS C:\> Get-PassedArgsRoot
PS C:\> Get-PassedArgsNoRoot
abc
PS C:\> $PSVersionTable

Name                           Value
----                           -----
BuildVersion                   3.0.0.0
WSManStackVersion              3.0
PSEdition                      Core
PSRemotingProtocolVersion      2.3
CLRVersion
SerializationVersion           1.1.0.1
GitCommitId                    v6.0.0-alpha.13
PSVersion                      6.0.0-alpha
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}

If this test loads the module from the file, would not generate it every time and put into the asserts folder?

The asserts folder should only keep special test data that cannot (or cannot easily) be generated on the fly. For simple test data like this, I think it's better to just setup the test data in the test.

Here I thought about the test performance only.

Since we do import-reload-remove for modules then would it be more purely do so in individual runspaces?

I agree. Will make the change.

On a second thought, I'm not sure if it's really worthy to run each It in an individual runspace. Here are my arguments:

1. it's slow to run each It in a separate runspace.
2. module reloading is supposed to work in this way no matter the runspace is clean or not, and it shouldn't pollute the runspace. Running the tests in a "not-so-clean" runspace provides extra benefit to see if it doesn't work properly in such case.

I agree. My only fear was that these tests can influence each other or be dependent on environment.

It shouldn't affect each other and pollute the runspace, but if we find so, say it causes instability to other tests, then we got another bug to fix 😄

👍