Conversation
| var preResult = await ConfigHandler.GetPreSocksCoreConfigContext(contextMod); | ||
| if (preResult is not null) | ||
| { | ||
| var validatorResult = preResult.ValidatorResult; |
There was a problem hiding this comment.
刚好这里也用到了,类似下面的代码,是否可以复用?
var msgs = new List<string>([..validatorResult.Errors, ..validatorResult.Warnings]);
if (msgs.Count > 0)
{
foreach (var msg in msgs)
{
NoticeManager.Instance.SendMessage(msg);
}
NoticeManager.Instance.Enqueue(Utils.List2String(msgs.Take(10).ToList(), true));
if (!validatorResult.Success)
{
return;
}
}
| var node = contextMod.Node; | ||
| var fileName = Utils.GetBinConfigPath(Global.CoreConfigFileName); | ||
| var preContext = ConfigHandler.GetPreSocksCoreConfigContext(contextMod); | ||
| var preResult = await ConfigHandler.GetPreSocksCoreConfigContext(contextMod); |
There was a problem hiding this comment.
GetPreSocksCoreConfigContext 是在这里获取的,
但是 var (context, validatorResult) = await CoreConfigContextBuilder.Build(_config, profileItem); 是在 MainWindowViewModel.Reload 获取 , 感觉有点不优雅了
|
那就都移到 CoreManager 里,然后写个函数复用 Result output? |
|
在这个 PR 之前,放 vm 中也行的。但是现在又多了一个 preContext ,那么应该都放到 CoreManager 更合理。 |
|
加了个 CoreConfigContextBuilder.BuildAll 和 NoticeManager.NotifyValidatorResult 统一在 vm 里处理 |
|
好的,合并 |
There was a problem hiding this comment.
Pull request overview
This PR addresses the TUN + sing-box startup failure reported in #8863 by ensuring the pre-service (sing-box) configuration is built using a correctly-resolved sing-box context, and by consolidating validator notifications to reduce duplicated UI messaging logic.
Changes:
- Introduces
CoreConfigContextBuilder.BuildAll(...)to build both the main context and an optional pre-socks context, and to provide a resolved main context with merged pre-socks ports. - Updates core startup flow to pass both main and pre contexts explicitly (
CoreManager.LoadCore(mainContext, preContext)), avoiding reusing an Xray-derived context for sing-box pre-service config generation. - Centralizes validator error/warning notification via
NoticeManager.NotifyValidatorResult(...)and replaces duplicated messaging blocks in view models.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| v2rayN/ServiceLib/ViewModels/ProfilesViewModel.cs | Replaces duplicated validator messaging with NoticeManager.NotifyValidatorResult. |
| v2rayN/ServiceLib/ViewModels/MainWindowViewModel.cs | Switches reload path to BuildAll and passes main+pre contexts into core loading. |
| v2rayN/ServiceLib/Manager/NoticeManager.cs | Adds NotifyValidatorResult helper to standardize validator notifications. |
| v2rayN/ServiceLib/Manager/CoreManager.cs | Changes LoadCore signature to accept main+pre contexts; generates main config from resolved context and starts optional pre-service. |
| v2rayN/ServiceLib/Handler/ConfigHandler.cs | Removes GetPreSocksCoreConfigContext (logic moved into CoreConfigContextBuilder). |
| v2rayN/ServiceLib/Handler/Builder/CoreConfigContextBuilder.cs | Adds BuildAll and pre-socks context construction so sing-box pre-service config is generated from a sing-box-appropriate context. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var protectDomainList = nodeContext.ProtectDomainList ?? []; | ||
| protectDomainList.UnionWith(preSocksResult.Context.ProtectDomainList ?? []); | ||
| return preSocksResult with | ||
| { | ||
| Context = preSocksResult.Context with { ProtectDomainList = protectDomainList } | ||
| }; |
There was a problem hiding this comment.
protectDomainList reuses nodeContext.ProtectDomainList and then assigns the same HashSet instance into preSocksResult.Context. This couples the main and pre-socks contexts via shared mutable state, which can lead to hard-to-track side effects if either context is modified later. Consider creating a new HashSet<string> copy for the merged domains and assigning copies (or at least avoiding sharing the same instance) to the contexts.
| var protectDomainList2 = nodeContext.ProtectDomainList ?? []; | ||
| protectDomainList2.UnionWith(preResult2.Context.ProtectDomainList ?? []); | ||
| return preResult2 with | ||
| { | ||
| Context = preResult2.Context with | ||
| { | ||
| ProtectDomainList = protectDomainList2, | ||
| TunProtectSsPort = tunProtectSsPort, | ||
| ProxyRelaySsPort = proxyRelaySsPort, | ||
| } |
There was a problem hiding this comment.
Similar to the pre-socks branch above: protectDomainList2 is taken directly from nodeContext.ProtectDomainList and then stored into preResult2.Context, so both contexts share the same mutable HashSet. Consider creating a new merged set (e.g., copy main + union pre) to keep the contexts independent and reduce surprise mutations.
There was a problem hiding this comment.
protectDomainList 和 protectDomainList2 ,最后结果会变成一样,是否这个是预期的效果?
如果不是,可以要深拷贝下再用
There was a problem hiding this comment.
算是吧,Protect 范围应该越大越好,并且两个核心保持一致
语义上好像确实有点歧义
|
修了 顺便,方便的话可以看下这个 #8841 (comment) |
Fix #8863
当启用 TUN 时,sing-box 负责分流,应以
RunCoreType=ECoreType.sing_box检查节点合法