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

Fix value sharing exception and deadlock #2882

Merged
merged 1 commit into from
Apr 6, 2023

Conversation

shyamnamboodiripad
Copy link
Contributor

@shyamnamboodiripad shyamnamboodiripad commented Apr 5, 2023

No description provided.

@@ -59,6 +59,7 @@ public static void RegisterDefaults()
{
_envelopeTypesByCommandTypeName = new ConcurrentDictionary<string, Type>
{
[nameof(AnonymousKernelCommand)] = typeof(KernelCommandEnvelope<AnonymousKernelCommand>),
Copy link
Contributor

@jonsequitur jonsequitur Apr 5, 2023

Choose a reason for hiding this comment

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

This should not be in the list. AnonymousKernelCommand isn't intended to cross process boundaries and can't be correctly serialized. It's a shim to run arbitrary code on the scheduler inline with other commands.

@@ -120,11 +120,38 @@ public async Task it_can_create_a_proxy_kernel_to_more_than_one_remote_subkernel
SupportedKernelCommands = fsharpKernelInfo.SupportedKernelCommands
};

const string installHttpKernel =
"""
#r "Microsoft.DotNet.Interactive.HttpRequest.dll"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unlikely to behave the way we expect it to in the context of the test project. We should discuss.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This does seem to work (including in CI) :) But will discuss offline.

…n by default.

This fixes a deadlock + exception.
@jonsequitur jonsequitur enabled auto-merge (rebase) April 6, 2023 01:02
@shyamnamboodiripad shyamnamboodiripad enabled auto-merge (squash) April 6, 2023 01:23
@shyamnamboodiripad shyamnamboodiripad merged commit 3a02ac4 into dotnet:main Apr 6, 2023
@colombod colombod added the bug Something isn't working label Apr 12, 2023
@shyamnamboodiripad shyamnamboodiripad deleted the fixvaluesharing branch April 25, 2023 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants