Skip to content

Commit 28f68cb

Browse files
benaclejamesbenaclejames
authored
Merge pull request #347 from hyblocker/fix/sandboxing-fixes
Sandboxing fixes
2 parents ac445d7 + 2908638 commit 28f68cb

4 files changed

Lines changed: 100 additions & 46 deletions

File tree

‎VRCFaceTracking.Core/Library/UnifiedLibManager.cs‎

Lines changed: 45 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ public void Initialize()
279279
{
280280
replyUpdatePacket.UpdateGlobalExpressionState();
281281
}
282+
replyUpdatePacket.UpdateHeadState();
282283
}
283284

284285
break;
@@ -445,45 +446,54 @@ private bool TeardownModuleSandboxed(ModuleRuntimeInfo module)
445446
_sandboxServer.SendData(eventTeardownPacket, module.SandboxProcessPort);
446447

447448
// Kill the update thread
448-
module.UpdateCancellationToken.Cancel();
449-
if ( module.UpdateThread.IsAlive )
450-
{
451-
// Edge case, we wait for the thread to finish before unloading the assembly
452-
_logger.LogDebug("Waiting for {module}'s thread to join...", module.ModuleClassName);
453-
module.UpdateThread.Join();
454-
}
455-
449+
module.UpdateCancellationToken?.Cancel();
456450
// Give the module 100ms to kill itself
457451
Thread.Sleep(100);
458452

459453
// Only bother tearing down a module if it's actually shutdown
460-
if ( !module.Process.HasExited )
454+
if ( !(module.Process?.HasExited ?? true) )
461455
{
462456
_logger.LogDebug("Module process has not yet exited");
463-
// @Note: Forcefully kill the process. We'll try to kill it 1000 times and then give up.
464-
int tries = 0;
465-
while ( tries < 1000 )
466-
{
467-
try
457+
try {
458+
if (!(module.Process?.WaitForExit(200) ?? false))
468459
{
469-
tries++;
470-
if ( !module.Process.HasExited )
471-
module.Process.Kill();
472-
if ( module.Process.HasExited )
460+
_logger.LogDebug("Module {id} didn't exit gracefully. Forcing kill...", module.Process?.Id ?? -1);
461+
module.Process?.Kill(entireProcessTree: true);
462+
if (!(module.Process?.WaitForExit(2000) ?? false))
473463
{
474-
tries = int.MaxValue;
475-
break;
464+
// on windows we can use taskkill /F /T /PID {procId} to force kill a process very aggressively. this has a higher success rate than process.kill!
465+
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) {
466+
using var killer = Process.Start(new ProcessStartInfo
467+
{
468+
FileName = "taskkill",
469+
Arguments = $"/F /T /PID {module.Process.Id}",
470+
CreateNoWindow = true,
471+
UseShellExecute = false
472+
});
473+
killer?.WaitForExit(2000);
474+
} else {
475+
_logger.LogCritical("Process {id} is a zombie or stuck in Kernel I/O. Manual intervention required.", module.Process.Id);
476+
}
477+
return false;
476478
}
477-
} catch ( System.ComponentModel.Win32Exception ex ) {
478-
// Can fail to call OpenProcessEx due to some error such as ACCESS_DENIED (process has higher priveleges, eg Sraniple)
479-
_logger.LogError($"Tried killing process with PID {module.Process.Id}. Got win32 error ({ex.ToString()}");
480-
} catch ( Exception ex ) {
481-
// Tell the user why we got an exception so that we can hopefully fix it.
482-
_logger.LogError($"Tried killing process with PID {module.Process.Id}. Got exception ({ex.HResult}) {ex.Message}");
483479
}
480+
} catch ( System.ComponentModel.Win32Exception ex ) {
481+
// Can fail to call OpenProcessEx due to some error such as ACCESS_DENIED (process has higher priveleges, eg Sraniple)
482+
_logger.LogError($"Tried killing process with PID {module.Process.Id}. Got win32 error ({ex.ToString()}");
483+
} catch ( Exception ex ) {
484+
// Tell the user why we got an exception so that we can hopefully fix it.
485+
_logger.LogError($"Tried killing process with PID {module.Process.Id}. Got exception ({ex.HResult}) {ex.Message}");
484486
}
485487
}
486488

489+
if (module.UpdateThread?.IsAlive ?? false)
490+
{
491+
// Edge case, we wait for the thread to finish before unloading the assembly
492+
var moduleName = module.ModuleInformation?.Name ?? module.ModuleClassName ?? "Unknown";
493+
_logger.LogDebug("Waiting for {module}'s thread to join...", moduleName);
494+
module.UpdateThread?.Join(500);
495+
}
496+
487497
return true;
488498
}
489499

@@ -495,6 +505,8 @@ public void TeardownAllAndResetAsync()
495505
foreach ( var module in _moduleThreads )
496506
{
497507
var success = false;
508+
if (module == null || (module.Process?.HasExited ?? true))
509+
continue;
498510
try
499511
{
500512
success = TeardownModuleSandboxed(module);
@@ -510,32 +522,26 @@ public void TeardownAllAndResetAsync()
510522

511523
_moduleThreads.Clear();
512524

513-
EyeStatus = ModuleState.Uninitialized;
514-
ExpressionStatus = ModuleState.Uninitialized;
515-
}
516-
517-
// Signal all active modules to gracefully shut down their respective runtimes
518-
public void TeardownAllAndResetAsyncLegacy()
519-
{
520-
_logger.LogInformation("Tearing down all modules...");
521-
522-
foreach ( var module in _moduleThreads )
525+
foreach ( var module in AvailableSandboxModules )
523526
{
524527
var success = false;
528+
if (module == null || (module.Process?.HasExited ?? true)) // c# objects may be null, use null coalesce to detect if a module has been destroyed but we have a lingering ref to it
529+
continue;
525530
try
526531
{
527532
success = TeardownModuleSandboxed(module);
528533
} finally
529534
{
530535
if ( !success )
531536
{
532-
_logger.LogWarning($"Module: {module.Module.ModuleInformation.Name} failed to shut down. Killing its thread.");
533-
module.UpdateThread.Interrupt();
537+
var moduleName = module.ModuleInformation?.Name ?? module.ModuleClassName ?? "Unknown";
538+
_logger.LogWarning($"Module: {moduleName} failed to shut down. Killing its thread.");
539+
module.UpdateThread?.Interrupt();
534540
}
535541
}
536542
}
537543

538-
_moduleThreads.Clear();
544+
AvailableSandboxModules.Clear();
539545

540546
EyeStatus = ModuleState.Uninitialized;
541547
ExpressionStatus = ModuleState.Uninitialized;

‎VRCFaceTracking.Core/Params/Data/Mutation/ParameterAdjustment.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,12 @@ public void Reset()
4545
[MutationProperty("Tongue Directions")] public (float, float) tongueMove = new(0, 1);
4646
[MutationProperty("Tongue Miscellaneous")] public (float, float) tongueOther = new(0, 1);
4747

48-
[MutationProperty("Head Rotation (Side-to-Side)")] public (float, float) headRotationYaw = new(0, 1);
49-
[MutationProperty("Head Rotation (Up-Down Tilt)")] public (float, float) headRotationPitch = new(0, 1);
50-
[MutationProperty("Head Rotation (Side Tilt)")] public (float, float) headRotationRoll = new(0, 1);
51-
[MutationProperty("Head Position (Side-to-Side)")] public (float, float) headPositionX = new(0, 1);
52-
[MutationProperty("Head Position (Up-Down)")] public (float, float) headPositionY = new(0, 1);
53-
[MutationProperty("Head Position (Forward-Back)")] public (float, float) headPositionZ = new(0, 1);
48+
[MutationProperty("Head Rotation (Side-to-Side)")] public (float, float) headRotationYaw = new(-1, 1);
49+
[MutationProperty("Head Rotation (Up-Down Tilt)")] public (float, float) headRotationPitch = new(-1, 1);
50+
[MutationProperty("Head Rotation (Side Tilt)")] public (float, float) headRotationRoll = new(-1, 1);
51+
[MutationProperty("Head Position (Side-to-Side)")] public (float, float) headPositionX = new(-1, 1);
52+
[MutationProperty("Head Position (Up-Down)")] public (float, float) headPositionY = new(-1, 1);
53+
[MutationProperty("Head Position (Forward-Back)")] public (float, float) headPositionZ = new(-1, 1);
5454

5555
public override string Name => "Parameter Adjustment";
5656

‎VRCFaceTracking.Core/Sandboxing/IPC/ReplyUpdatePacket.cs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,14 @@ internal class UpdateDataContiguous
2929
internal float Eye_Right_PupilDiameter_MM;
3030
internal float Eye_Right_Openness;
3131

32+
internal float Head_Yaw;
33+
internal float Head_Pitch;
34+
internal float Head_Roll;
35+
36+
internal float Head_PosX;
37+
internal float Head_PosY;
38+
internal float Head_PosZ;
39+
3240
[MarshalAs(UnmanagedType.ByValArray, SizeConst = EXPRESSION_COUNT)]
3341
internal float[] Expression_Shapes;
3442
}
@@ -64,6 +72,14 @@ public override byte[] GetBytes()
6472
_contiguousUnifiedData.Eye_MaxDilation = UnifiedTracking.Data.Eye._maxDilation;
6573
_contiguousUnifiedData.Eye_MinDilation = UnifiedTracking.Data.Eye._minDilation;
6674

75+
_contiguousUnifiedData.Head_Yaw = UnifiedTracking.Data.Head.HeadPitch;
76+
_contiguousUnifiedData.Head_Pitch = UnifiedTracking.Data.Head.HeadRoll;
77+
_contiguousUnifiedData.Head_Roll = UnifiedTracking.Data.Head.HeadYaw;
78+
79+
_contiguousUnifiedData.Head_PosX = UnifiedTracking.Data.Head.HeadPosX;
80+
_contiguousUnifiedData.Head_PosY = UnifiedTracking.Data.Head.HeadPosY;
81+
_contiguousUnifiedData.Head_PosZ = UnifiedTracking.Data.Head.HeadPosZ;
82+
6783
// Copy face tracking
6884
for ( int i = 0; i < _contiguousUnifiedData.Expression_Shapes.Length; i++ )
6985
{
@@ -153,6 +169,23 @@ public void UpdateGlobalEyeState()
153169
UnifiedTracking.Data.Eye._minDilation = _contiguousUnifiedData.Eye_MinDilation;
154170
}
155171

172+
public void UpdateHeadState()
173+
{
174+
if ( _contiguousUnifiedData.Head_Yaw != INVALID_FLOAT )
175+
UnifiedTracking.Data.Head.HeadYaw = _contiguousUnifiedData.Head_Yaw;
176+
if ( _contiguousUnifiedData.Head_Pitch != INVALID_FLOAT )
177+
UnifiedTracking.Data.Head.HeadPitch = _contiguousUnifiedData.Head_Pitch;
178+
if ( _contiguousUnifiedData.Head_Roll != INVALID_FLOAT )
179+
UnifiedTracking.Data.Head.HeadRoll = _contiguousUnifiedData.Head_Roll;
180+
181+
if ( _contiguousUnifiedData.Head_PosX != INVALID_FLOAT )
182+
UnifiedTracking.Data.Head.HeadPosX = _contiguousUnifiedData.Head_PosX;
183+
if ( _contiguousUnifiedData.Head_PosY != INVALID_FLOAT )
184+
UnifiedTracking.Data.Head.HeadPosY = _contiguousUnifiedData.Head_PosY;
185+
if ( _contiguousUnifiedData.Head_PosZ != INVALID_FLOAT )
186+
UnifiedTracking.Data.Head.HeadPosZ = _contiguousUnifiedData.Head_PosZ;
187+
}
188+
156189
public void UpdateGlobalExpressionState()
157190
{
158191
// Copy face tracking

‎VRCFaceTracking.Core/Utils.cs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,22 @@ public static void KillAllProcessesOfName(string name)
7070

7171
try
7272
{
73-
proc.Kill();
73+
proc.Kill(entireProcessTree: true);
74+
if (!proc.WaitForExit(2000))
75+
{
76+
// on windows we can use taskkill /F /T /PID {procId} to force kill a process very aggressively. this has a higher success rate than process.kill!
77+
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) {
78+
using var killer = Process.Start(new ProcessStartInfo
79+
{
80+
FileName = "taskkill",
81+
Arguments = $"/F /T /PID {proc.Id}",
82+
CreateNoWindow = true,
83+
UseShellExecute = false
84+
});
85+
killer?.WaitForExit(2000);
86+
}
87+
}
88+
7489
}
7590
catch
7691
{

0 commit comments

Comments
 (0)