Skip to content
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

[wasm] Mark System.Threading.ThreadPool unsupported on Browser #41891

Merged

Conversation

mdh1418
Copy link
Member

@mdh1418 mdh1418 commented Sep 4, 2020

Contributes to #41087

M:System.Threading.ThreadPool.BindHandle(System.IntPtr),System.Threading,ThreadPool,BindHandle(IntPtr),0
"M:System.Threading.ThreadPool.UnsafeRegisterWaitForSingleObject(System.Threading.WaitHandle,System.Threading.WaitOrTimerCallback,System.Object,System.Int32,System.Boolean)",System.Threading,ThreadPool,"UnsafeRegisterWaitForSingleObject(WaitHandle, WaitOrTimerCallback, Object, Int32, Boolean)",1
"M:System.Threading.ThreadPool.RegisterWaitForSingleObject(System.Threading.WaitHandle,System.Threading.WaitOrTimerCallback,System.Object,System.UInt32,System.Boolean)",System.Threading,ThreadPool,"RegisterWaitForSingleObject(WaitHandle, WaitOrTimerCallback, Object, UInt32, Boolean)",1
"M:System.Threading.ThreadPool.UnsafeRegisterWaitForSingleObject(System.Threading.WaitHandle,System.Threading.WaitOrTimerCallback,System.Object,System.Int64,System.Boolean)",System.Threading,ThreadPool,"UnsafeRegisterWaitForSingleObject(WaitHandle, WaitOrTimerCallback, Object, Int64, Boolean)",1
"M:System.Threading.ThreadPool.RegisterWaitForSingleObject(System.Threading.WaitHandle,System.Threading.WaitOrTimerCallback,System.Object,System.Int32,System.Boolean)",System.Threading,ThreadPool,"RegisterWaitForSingleObject(WaitHandle, WaitOrTimerCallback, Object, Int32, Boolean)",1
"M:System.Threading.ThreadPool.RegisterWaitForSingleObject(System.Threading.WaitHandle,System.Threading.WaitOrTimerCallback,System.Object,System.Int64,System.Boolean)",System.Threading,ThreadPool,"RegisterWaitForSingleObject(WaitHandle, WaitOrTimerCallback, Object, Int64, Boolean)",1
"M:System.Threading.ThreadPool.UnsafeRegisterWaitForSingleObject(System.Threading.WaitHandle,System.Threading.WaitOrTimerCallback,System.Object,System.UInt32,System.Boolean)",System.Threading,ThreadPool,"UnsafeRegisterWaitForSingleObject(WaitHandle, WaitOrTimerCallback, Object, UInt32, Boolean)",1
M:System.Threading.ThreadPool.BindHandle(System.Runtime.InteropServices.SafeHandle),System.Threading,ThreadPool,BindHandle(SafeHandle),0
"M:System.Threading.ThreadPool.RegisterWaitForSingleObject(System.Threading.WaitHandle,System.Threading.WaitOrTimerCallback,System.Object,System.TimeSpan,System.Boolean)",System.Threading,ThreadPool,"RegisterWaitForSingleObject(WaitHandle, WaitOrTimerCallback, Object, TimeSpan, Boolean)",1
"M:System.Threading.ThreadPool.UnsafeRegisterWaitForSingleObject(System.Threading.WaitHandle,System.Threading.WaitOrTimerCallback,System.Object,System.TimeSpan,System.Boolean)",System.Threading,ThreadPool,"UnsafeRegisterWaitForSingleObject(WaitHandle, WaitOrTimerCallback, Object, TimeSpan, Boolean)",1
M:System.Threading.RegisteredWaitHandle.Unregister(System.Threading.WaitHandle),System.Threading,RegisteredWaitHandle,Unregister(WaitHandle),0

@ghost
Copy link

ghost commented Sep 4, 2020

Tagging subscribers to this area: @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

@@ -32,6 +33,7 @@ public static bool BindHandle(IntPtr osHandle)
throw new PlatformNotSupportedException(SR.Arg_PlatformNotSupported); // Replaced by ThreadPoolBoundHandle.BindHandle
}

[UnsupportedOSPlatform("browser")]
public static bool BindHandle(SafeHandle osHandle)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be added to

public static bool BindHandle(SafeHandle osHandle)
as well

@mdh1418 mdh1418 force-pushed the mdhwang/system_threading_threadpool_unsupported branch from 0b32bab to 7d07b8c Compare September 4, 2020 21:09
@@ -32,6 +33,7 @@ public static bool BindHandle(IntPtr osHandle)
throw new PlatformNotSupportedException(SR.Arg_PlatformNotSupported); // Replaced by ThreadPoolBoundHandle.BindHandle
}

[UnsupportedOSPlatform("browser")]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it looks like ThreadPool.BindHandle is not supported on any of mono, I wonder if it would be better to add support for UnsupportedOSPlatform("mono") or to list UnsupportedOSPlatform attribute for each mono platform for now. I believe having just UnsupportedOSPlatform("browser") would imply that this API is supported on the other mono platforms.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this API is Windows specific (cc @jkotas for confirmation) and we could mark it that way. Using UnsupportedOSPlatform("mono") combination does not make sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, BindHandle is Windows-specific. CoreCLR implementation throws PNSE on Unix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdh1418 could you turn the attribute to [SupportedOSPlatform("windows")]

@mdh1418 mdh1418 merged commit ac847fc into dotnet:master Sep 8, 2020
@mdh1418 mdh1418 deleted the mdhwang/system_threading_threadpool_unsupported branch September 8, 2020 23:24
mdh1418 added a commit to mdh1418/runtime that referenced this pull request Sep 8, 2020
…t#41891)

* System.Threading.ThreadPool enable platform attributes

* Mark System.Threading.ThreadPool APIs unsupported on browser

* System.Threading.ThreadPool Add Unsupported attribute to other ThreadPool files

* Remove Unsupported attributes from BindHandle

* Add windows Supported Attribute to BindHandle

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>
mdh1418 added a commit that referenced this pull request Sep 9, 2020
* [wasm] Mark System.ComponentModel APIs as unsupported on Browser (#41094)

* [wasm] System.ComponentModel enable platform attributes

* [wasm] Mark ExtendedProtectionPolicyTypeConverter.ConvertTo as unsupported

* [wasm] Mark System.ComponentModel.TypeDescriptor.CreateInstance as unsupported

* [wasm] Mark System.ComponentModel.TypeDescriptionProvider.CreateInstance as unsupported

* [wasm] Mark System.ComponentModel.LicenseManager.CreateWithContext as unsupported

* [wasm] Mark System.ComponentModel.MaskedTextProvider.Clone as unsupported

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>

* [wasm] Mark System.Console APIs as unsupported on Browser (#41184)

* [wasm] Mark System.Console APIs as unsupported on Browser

* System.Console remove unsupported attribute from Console.Clear()

* System.Console mark SetIn as unsupported on Browser

* System.Console move UnsupportedOSPlatform attributes down to the accessor

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>

* Mark System.Diagnostics.FileVersionInfo as unsupported on Browser (#41271)

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>

* Mark System.Diagnostics.Process unsupported at assembly level (#41694)

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>

* [wasm] Mark System.IO.Compression APIs as unsupported on Browser (#41683)

* [wasm] System.IO.Compression.Brotli enable platform attributes

* Mark System.IO.Compression.Brotli unsupported at assembly level

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>

* [wasm] Mark System.IO.FileSystem.Watcher APIs as unsupported on Browser (#41682)

* [wasm] System.IO.FileSystem.Watcher enable platform attributes

* Mark System.IO.FileSystem.Watcher unsupported at assembly level

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>

* [wasm] Mark System.IO.IsolatedStorage APIs as unsupported on Browser (#41700)

* [wasm] System.IO.IsolatedStorage enable platform attributes

* Mark System.IO.IsolatedStorage Unsupported at assembly level

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>

* Mark some System.Net.* APIs as unsupported on Browser WASM (#40924)

* [wasm] Mark System.Threading.ThreadPool unsupported on Browser (#41891)

* System.Threading.ThreadPool enable platform attributes

* Mark System.Threading.ThreadPool APIs unsupported on browser

* System.Threading.ThreadPool Add Unsupported attribute to other ThreadPool files

* Remove Unsupported attributes from BindHandle

* Add windows Supported Attribute to BindHandle

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>

* Mark System.Net.WebSockets.ClientWebSocketOptions APIs as unsupported on Browser (#41963)

* Mark System.Net.WebSockets.ClientWebSocketOptions APIs as unsupported on Browser

* Add the attributes to non-browser version of ClientWebSocketOptions class to avoid build error

* Add using

* Include platform attributes

* [wasm] Mark System.Net.NameResolution Unsupported at assembly level (#41985)

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>
Co-authored-by: Maxim Lipnin <v-maxlip@microsoft.com>
mdh1418 added a commit to mdh1418/runtime that referenced this pull request Sep 11, 2020
…t#41891)

* System.Threading.ThreadPool enable platform attributes

* Mark System.Threading.ThreadPool APIs unsupported on browser

* System.Threading.ThreadPool Add Unsupported attribute to other ThreadPool files

* Remove Unsupported attributes from BindHandle

* Add windows Supported Attribute to BindHandle

Co-authored-by: Mitchell Hwang <mitchell.hwang@microsoft.com>
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Threading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants