-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix broken WSLCorePort channel after receive timeout #14455
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
base: master
Are you sure you want to change the base?
Changes from all commits
df250f5
23c189e
9320322
c283118
69f4d11
9aaaf93
16d2043
8aaa647
e28da89
19078fd
e46e903
5b21021
458df6f
fe9a05b
a71e2a4
32c1586
cee5ae5
d96a7b1
864e383
84aad97
a59098f
a4af7b6
4959e90
504cb18
b846ace
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 |
|---|---|---|
|
|
@@ -334,7 +334,7 @@ try | |
| CATCH_LOG() | ||
|
|
||
| void ConfigHandleInteropMessage( | ||
| wsl::shared::SocketChannel& ResponseChannel, | ||
| wsl::shared::Transaction& Transaction, | ||
| wsl::shared::SocketChannel& InteropChannel, | ||
| bool Elevated, | ||
| gsl::span<gsl::byte> Message, | ||
|
|
@@ -350,7 +350,7 @@ Routine Description: | |
|
|
||
| Arguments: | ||
|
|
||
| ResponseChannel - Supplies channel used to send responses. | ||
| Transaction - Supplies transaction used to send responses. | ||
|
|
||
| InteropChannel - Supplies a channel to the host to be used for create | ||
| process requests. | ||
|
|
@@ -381,7 +381,7 @@ try | |
|
|
||
| case LxInitMessageQueryDrvfsElevated: | ||
| { | ||
| ResponseChannel.SendResultMessage<bool>(Elevated); | ||
| Transaction.SendResultMessage<bool>(Elevated); | ||
| break; | ||
| } | ||
|
|
||
|
|
@@ -397,15 +397,15 @@ try | |
| auto Value = UtilGetEnvironmentVariable(Query->Buffer); | ||
| wsl::shared::MessageWriter<LX_INIT_QUERY_ENVIRONMENT_VARIABLE> Response(LxInitMessageQueryEnvironmentVariable); | ||
| Response.WriteString(Value); | ||
| ResponseChannel.SendMessage<LX_INIT_QUERY_ENVIRONMENT_VARIABLE>(Response.Span()); | ||
| Transaction.Send<LX_INIT_QUERY_ENVIRONMENT_VARIABLE>(Response.Span()); | ||
| } | ||
|
|
||
| break; | ||
|
|
||
| case LxInitMessageQueryFeatureFlags: | ||
| { | ||
| assert(Config.FeatureFlags.has_value()); | ||
| ResponseChannel.SendResultMessage<int32_t>(Config.FeatureFlags.value()); | ||
| Transaction.SendResultMessage<int32_t>(Config.FeatureFlags.value()); | ||
| break; | ||
| } | ||
|
|
||
|
|
@@ -419,7 +419,7 @@ try | |
| } | ||
|
|
||
| bool success = false; | ||
| auto sendResponse = wil::scope_exit([&]() { ResponseChannel.SendResultMessage<bool>(success); }); | ||
| auto sendResponse = wil::scope_exit([&]() { Transaction.SendResultMessage<bool>(success); }); | ||
|
|
||
| if (!Config.BootInit || Config.InitPid.value_or(0) != getpid()) | ||
| { | ||
|
|
@@ -435,7 +435,7 @@ try | |
|
|
||
| case LxInitMessageQueryNetworkingMode: | ||
| assert(Config.NetworkingMode.has_value()); | ||
| ResponseChannel.SendResultMessage<uint8_t>(static_cast<uint8_t>(Config.NetworkingMode.value())); | ||
| Transaction.SendResultMessage<uint8_t>(static_cast<uint8_t>(Config.NetworkingMode.value())); | ||
| break; | ||
|
|
||
| case LxInitMessageQueryVmId: | ||
|
|
@@ -446,7 +446,7 @@ try | |
| Response.WriteString(Config.VmId.value()); | ||
| } | ||
|
|
||
| ResponseChannel.SendMessage<LX_INIT_QUERY_VM_ID>(Response.Span()); | ||
| Transaction.Send<LX_INIT_QUERY_VM_ID>(Response.Span()); | ||
| break; | ||
| } | ||
|
|
||
|
|
@@ -618,7 +618,7 @@ try | |
| } | ||
| CATCH_LOG() | ||
|
|
||
| int ConfigInitializeInstance(wsl::shared::SocketChannel& Channel, gsl::span<gsl::byte> Buffer, wsl::linux::WslDistributionConfig& Config) | ||
| int ConfigInitializeInstance(const std::function<void(const gsl::span<gsl::byte>&)>& SendResponse, gsl::span<gsl::byte> Buffer, wsl::linux::WslDistributionConfig& Config) | ||
|
Collaborator
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. What's the reason for taking an std::function instead of a reference to the transaction ?
Contributor
Author
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. This function is also used for WSL1, where the transaction is initiated with UtilReadMessageLxBus. Which does not work with the new transaction model. |
||
|
|
||
| /*++ | ||
|
|
||
|
|
@@ -632,7 +632,7 @@ Routine Description: | |
|
|
||
| Arguments: | ||
|
|
||
| MessageFd - Supplies a file descriptor to send the response message. | ||
| SendResponse - Supplies a function to send the response message. | ||
|
|
||
| Buffer - Supplies the message buffer. | ||
|
|
||
|
|
@@ -923,7 +923,7 @@ try | |
| Response.WriteString(Response->VersionIndex, Version->c_str()); | ||
| } | ||
|
|
||
| Channel.SendMessage<LX_INIT_CONFIGURATION_INFORMATION_RESPONSE>(Response.Span()); | ||
| SendResponse(Response.Span()); | ||
|
|
||
| // | ||
| // Accept the interop connection. | ||
|
|
@@ -973,13 +973,14 @@ try | |
| continue; | ||
| } | ||
|
|
||
| auto [Message, Span] = ClientChannel.ReceiveMessageOrClosed<MESSAGE_HEADER>(); | ||
| auto transaction = ClientChannel.ReceiveTransaction(); | ||
| auto [Message, Span] = transaction.ReceiveOrClosed<MESSAGE_HEADER>(); | ||
| if (Message == nullptr) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| ConfigHandleInteropMessage(ClientChannel, InteropChannel, Elevated, Span, Message, Config); | ||
| ConfigHandleInteropMessage(transaction, InteropChannel, Elevated, Span, Message, Config); | ||
| } | ||
| }); | ||
|
|
||
|
|
@@ -2186,7 +2187,7 @@ Return Value: | |
| return Result; | ||
| } | ||
|
|
||
| int ConfigRemountDrvFs(gsl::span<gsl::byte> Buffer, wsl::shared::SocketChannel& Channel, const wsl::linux::WslDistributionConfig& Config) | ||
| int ConfigRemountDrvFs(gsl::span<gsl::byte> Buffer, wsl::shared::Transaction& Transaction, const wsl::linux::WslDistributionConfig& Config) | ||
|
|
||
| /*++ | ||
|
|
||
|
|
@@ -2207,7 +2208,7 @@ Return Value: | |
|
|
||
| --*/ | ||
| { | ||
| Channel.SendResultMessage<int32_t>(ConfigRemountDrvFsImpl(Buffer, Config)); | ||
| Transaction.SendResultMessage<int32_t>(ConfigRemountDrvFsImpl(Buffer, Config)); | ||
|
|
||
| return 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.
if
gnsis running is test mode,channelwould be an empty channel here. I'm not sure that we should allow transaction on empty channels since that could lead to some weird states, maybe the easiest way here would be to make the transaction an std::optional, and plumb the optional throughNotificationRoutineandStatusRoutineso we don't have to deal with it in this classThere 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.
Creating a transaction on an empty channel is benign. It will not disrupt that channel's state. And using that transaction will behave the same as calling receive or send on that empty channel. The test path in StartGns already correctly ignores the input Transaction.
I feel that making the created transaction optional is too complicated for normal callers. Maybe we should keep it as is for this.