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

[BUG] EventExecutor exception #358

Open
askpt opened this issue Jan 31, 2025 · 2 comments
Open

[BUG] EventExecutor exception #358

askpt opened this issue Jan 31, 2025 · 2 comments
Labels
bug Something isn't working help wanted Extra attention is needed tests Improvement or additions to test suite

Comments

@askpt
Copy link
Member

askpt commented Jan 31, 2025

Description

We are currently experiencing some flaky unit tests that we should tackle. Example output:

Unhandled exception. System.Threading.Channels.ChannelClosedException: The channel has been closed.
   at OpenFeature.EventExecutor.ProcessFeatureProviderEventsAsync(Object providerRef) in /_/src/OpenFeature/EventExecutor.cs:line 238
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_1(Object state)
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading.PortableThreadPool.WorkerThread.WorkerThreadStart()
The active test run was aborted. Reason: Test host process crashed : Unhandled exception. System.Threading.Channels.ChannelClosedException: The channel has been closed.
   at OpenFeature.EventExecutor.ProcessFeatureProviderEventsAsync(Object providerRef) in /_/src/OpenFeature/EventExecutor.cs:line 238
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_1(Object state)
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading.PortableThreadPool.WorkerThread.WorkerThreadStart()


Test Run Aborted.
Total tests: Unknown
     Passed: 90
 Total time: 2.8570 Seconds
     6>_VSTestConsole:
         MSB4181: The "VSTestTask" task returned false but did not log an error.
     6>Done Building Project "/home/runner/work/dotnet-sdk/dotnet-sdk/test/OpenFeature.Tests/OpenFeature.Tests.csproj" (VSTest target(s)) -- FAILED.

As part of the debugging, we can try using the dotnet test—- blame to understand what is wrong.

Notes

@askpt askpt added bug Something isn't working help wanted Extra attention is needed tests Improvement or additions to test suite labels Jan 31, 2025
@kinyoklion
Copy link
Member

This looks to be a bug in the EventExecutor rather than just a flaky test. Writing to a channel will throw if the channel has been closed.

There is a purple note about it here: https://learn.microsoft.com/en-us/dotnet/core/extensions/channels#bounding-strategies

There isn't any synchronization between shutdown and the event processing loop.

            while (await reader.WaitToReadAsync().ConfigureAwait(false))
            {
                if (!reader.TryRead(out var item))
                    continue;

//!!!!!!!!!!!!!!! If you are executing any of the following, and ShutDownAsync is called or a providerr is registered, then you will get an exception.
                switch (item)
                {
                    case ProviderEventPayload eventPayload:
                        this.UpdateProviderStatus(typedProviderRef, eventPayload);
                        await this.EventChannel.Writer.WriteAsync(new Event { Provider = typedProviderRef, EventPayload = eventPayload }).ConfigureAwait(false);
                        break;
                }
            }

Theoretically this could happen in real situations and not jsut a test.

It seems the typical pattern is for the producer to close the channel after it has exhausted things it wants to produce.

So, typically I think that writes and completing the channel would be happening either on the same thread, or would be synchronized.

In our case though I think we could just handle the exception from the WriteAsync.

@askpt askpt changed the title [BUG] Flaky Unit Tests [BUG] EventExecutor exception Feb 24, 2025
@askpt
Copy link
Member Author

askpt commented Feb 24, 2025

This looks to be a bug in the EventExecutor rather than just a flaky test. Writing to a channel will throw if the channel has been closed.

There is a purple note about it here: https://learn.microsoft.com/en-us/dotnet/core/extensions/channels#bounding-strategies

There isn't any synchronization between shutdown and the event processing loop.

            while (await reader.WaitToReadAsync().ConfigureAwait(false))
            {
                if (!reader.TryRead(out var item))
                    continue;

//!!!!!!!!!!!!!!! If you are executing any of the following, and ShutDownAsync is called or a providerr is registered, then you will get an exception.
                switch (item)
                {
                    case ProviderEventPayload eventPayload:
                        this.UpdateProviderStatus(typedProviderRef, eventPayload);
                        await this.EventChannel.Writer.WriteAsync(new Event { Provider = typedProviderRef, EventPayload = eventPayload }).ConfigureAwait(false);
                        break;
                }
            }

Theoretically this could happen in real situations and not jsut a test.

It seems the typical pattern is for the producer to close the channel after it has exhausted things it wants to produce.

So, typically I think that writes and completing the channel would be happening either on the same thread, or would be synchronized.

In our case though I think we could just handle the exception from the WriteAsync.

@kinyoklion Thanks for investigating this. I changed the title and description to better align with your findings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed tests Improvement or additions to test suite
Projects
None yet
Development

No branches or pull requests

2 participants