-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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.
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
mark FileStream.Lock and Unlock as unsupported on macOS #47040
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
using System.Diagnostics; | ||
using System.Runtime.InteropServices; | ||
using System.Runtime.Serialization; | ||
using System.Runtime.Versioning; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.Win32.SafeHandles; | ||
|
@@ -256,6 +257,7 @@ public FileStream(string path, FileMode mode, FileAccess access, FileShare share | |
[Obsolete("This property has been deprecated. Please use FileStream's SafeFileHandle property instead. https://go.microsoft.com/fwlink/?linkid=14202")] | ||
public virtual IntPtr Handle => SafeFileHandle.DangerousGetHandle(); | ||
|
||
[UnsupportedOSPlatform("macos")] | ||
public virtual void Lock(long position, long length) | ||
{ | ||
if (position < 0 || length < 0) | ||
|
@@ -271,6 +273,7 @@ public virtual void Lock(long position, long length) | |
LockInternal(position, length); | ||
} | ||
|
||
[UnsupportedOSPlatform("macos")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Presumably the ref needs to be updated, too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, what about overrides? e.g. IsolatedStorageFileStream.Lock/Unlock? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stephentoub excellent points (as usual). I've added missing attributes (ref & overrides) PTAL There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am sorry, I've missed the VB files... let me fix the build and get back to you |
||
public virtual void Unlock(long position, long length) | ||
{ | ||
if (position < 0 || length < 0) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this API's area owner would prefer to reduce the aggressiveness of marking the whole platform as unsupported, and instead only calling
oFile.Lock()
andoFile.Unlock()
when the platform is not MacOS.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know, mono team has a story for adding more attributes for mac/android/ios, they might want to see this changes cc @marek-safar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That story is for SDKs but we are going to audit libraries as well during #47910 work