Skip to content

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Jan 12, 2020

Fixes #1646

@jkotas
Copy link
Member Author

jkotas commented Jan 12, 2020

This is technically a breaking change, but I believe it is better to throw than silently do the wrong thing. It is unlikely that a working program depends on the current broken behavior.

If we agree that that this is a good change to make, I am also going to send PR to update the docs.

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

I was surprised by the lack of exception. I think this is worth changing.

@NN---
Copy link
Contributor

NN--- commented Jan 12, 2020

Why breaking ?
On some implementations, if more that 64 handles are passed, a NotSupportedException is thrown.

https://docs.microsoft.com/en-us/dotnet/api/system.threading.waithandle.waitany?view=netframework-4.8#System_Threading_WaitHandle_WaitAny_System_Threading_WaitHandle___

@stephentoub
Copy link
Member

stephentoub commented Jan 12, 2020

Why breaking ?

On an STA thread, passing in 64 handles to WaitAny currently results in it immediately returning a timeout error code. This change makes it throw.

@NN---
Copy link
Contributor

NN--- commented Jan 12, 2020

Is it documented somewhere?

@jkotas jkotas added the breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. label Jan 12, 2020
@jkotas
Copy link
Member Author

jkotas commented Jan 13, 2020

Is it documented somewhere?

It is not that's part of the problem. I am updating the documentation in dotnet/dotnet-api-docs#3767

@jkotas jkotas merged commit d74ea2f into dotnet:master Jan 13, 2020
new ManualResetEvent(true),
new ManualResetEvent(true)
};
ManualResetEvent[] handles = CreateManualResetEvents(3);
Copy link
Member

Choose a reason for hiding this comment

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

I guess disposing these isn’t important

@jkotas jkotas deleted the waithandle branch January 14, 2020 07:33
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Threading breaking-change Issue or PR that represents a breaking API or functional change over a prerelease.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WaitHandle.WaitAny with 64 handle on STA threads

4 participants