You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Property setter assigning to field in HyperVSocketEndPoint #13995
This looks like a typo bug to me. The only effect appears to be that VmId and ServiceId values are not unique. Since this class can be serialized I have to assume the information may be persisted, and fixing the typo may cause a breaking change.
Given how this property setter works, I don't think it's possible for ServiceId to be deserialised correctly currently... Currently the effect would appear to be that the underlying data for ServiceId will always and only ever be the default value, unless you manually set the backing field. Any attempt to set the value by the property will simply set the VM ID instead.
The Equals() method on this class is only comparing ServiceId values via the VmId field which is overwritten with the ServiceId value. The ServiceId field appears to always contain Guid.Empty. So if if two HyperVSocketEndPoint objects have the same ServiceId but different VmId values they will compare the same. Probably not a good situation.
This is obviously wrong, and I feel we should take the fix. There may be a small possibility of regression but I feel it is minimal and worth taking the fix. I ran this by the committee and they felt the same.
Activity
vexx32 commentedon Nov 6, 2020
/cc Paul Higinbotham (@PaulHigin)
iSazonov commentedon Nov 6, 2020
It comes from Windows PowerShell. If it is not bug we need to add a comment in the code.
It seems was Steve Lee (@SteveL-MSFT) who communicated with Hyper-V team in last time.
Suppress IDE0044
xtqqczze commentedon Jun 27, 2021
Paul Higinbotham (@PaulHigin) Steve Lee (@SteveL-MSFT) Can someone look at this please.
PaulHigin commentedon Jun 29, 2021
This looks like a typo bug to me. The only effect appears to be that VmId and ServiceId values are not unique. Since this class can be serialized I have to assume the information may be persisted, and fixing the typo may cause a breaking change.
vexx32 commentedon Jun 29, 2021
Given how this property setter works, I don't think it's possible for
ServiceIdto be deserialised correctly currently... Currently the effect would appear to be that the underlying data forServiceIdwill always and only ever be the default value, unless you manually set the backing field. Any attempt to set the value by the property will simply set the VM ID instead.rkeithhill commentedon Jun 30, 2021
The
Equals()method on this class is only comparingServiceIdvalues via theVmIdfield which is overwritten with theServiceIdvalue. TheServiceIdfield appears to always containGuid.Empty. So if if twoHyperVSocketEndPointobjects have the sameServiceIdbut differentVmIdvalues they will compare the same. Probably not a good situation.PaulHigin commentedon Jun 30, 2021
This is obviously wrong, and I feel we should take the fix. There may be a small possibility of regression but I feel it is minimal and worth taking the fix. I ran this by the committee and they felt the same.
vexx32 commentedon Jun 30, 2021
Putting it up for grabs for now, as it seems a simple enough change for community folks to take on if they would like.