Switch to using named pipes for IPC #6442
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 tasks
|
Need some build inspection issues fixed here. |
|
Yeah having issues with rider showing hundreds of warnings at the moment, bit frustrating. |
smoogipoo
approved these changes
Dec 6, 2024
peppy
commented
Dec 6, 2024
| @@ -146,8 +146,6 @@ public abstract class GameHost : IIpcHost, IDisposable | |||
| public virtual Task SendMessageAsync(IpcMessage message) => throw new NotSupportedException("This platform does not implement IPC."); | |||
| public virtual Task<IpcMessage> SendMessageWithResponseAsync(IpcMessage message) => throw new NotSupportedException("This platform does not implement IPC."); | |||
|
|
|||
| public virtual Task SendM(IpcMessage message) => throw new NotSupportedException("This platform does not implement IPC."); | |||
peppy
added a commit
to peppy/osu-framework
that referenced
this pull request
Dec 11, 2024
Regressed with ppy#6442. Was not visible in test coverage because as per the inline comment, this is an exceptional case where two different processes bind. I'm not sure how to add test coverage for this, but I have tested manually on both windows and macOS.
peppy
added a commit
to peppy/osu-framework
that referenced
this pull request
Dec 11, 2024
Regressed with ppy#6442. Was not visible in test coverage because as per the inline comment, this is an exceptional case where two different processes bind. I'm not sure how to add test coverage for this, but I have tested manually on both windows and macOS.
peppy
added a commit
to peppy/osu-framework
that referenced
this pull request
Dec 11, 2024
Regressed with ppy#6442. Was not visible in test coverage because as per the inline comment, this is an exceptional case where two different processes bind. I'm not sure how to add test coverage for this, but I have tested manually on both windows and macOS.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves ppy/osu#30783.
Has come up a few times in recent months so worthwhile fixing.
I've used this opportunity to also make the API a touch more robust and lock in the bidirectional messaging API too.
Breaking Changes
osu!framework now uses named pipes for IPC instead of TCP ports
This resolves cases where other applications may be reserving ranges of ports on localhost (as we found is common of WSL). Things should still work out-of-the-box if you are already using IPC, but you may want to update your usage as we've obsoleted the port specification:
In addition, there's now a new
SendMessageWithResponseAsyncexposed byGameHostwhich can be used for bidirectional messaging. If you wish to make use of this, you should make anIpcChannel<T,TResponse>with well defined types for the sending and responding messages.