Skip to content

recursion checker proof-of-concept #4560

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

recursion checker proof-of-concept #4560

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 22 additions & 22 deletions com.unity.ml-agents/Runtime/Academy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,26 @@

namespace Unity.MLAgents
{
internal class RecursionChecker : IDisposable
{
private bool m_IsRunning;

public IDisposable Start()
{
if (m_IsRunning)
{
throw new UnityAgentsException("Don't do this.");
}
m_IsRunning = true;
return this;
}

public void Dispose()
{
m_IsRunning = false;
}
}

/// <summary>
/// Helper class to step the Academy during FixedUpdate phase.
/// </summary>
Expand Down Expand Up @@ -157,7 +177,7 @@ public bool IsCommunicatorOn

// Whether the Academy is in the middle of a step. This is used to detect and Academy
// step called by user code that is also called by the Academy.
bool m_IsStepping;
private RecursionChecker m_StepRecursionChecker = new RecursionChecker();

// Random seed used for inference.
int m_InferenceSeed;
Expand Down Expand Up @@ -535,22 +555,7 @@ void ForcedFullReset()
/// </summary>
public void EnvironmentStep()
{
// Check whether we're already in the middle of a step.
// This shouldn't happen generally, but could happen if user code (e.g. CollectObservations)
// that is called by EnvironmentStep() also calls EnvironmentStep(). This would result
// in an infinite loop and/or stack overflow, so stop it before it happens.
if (m_IsStepping)
{
throw new UnityAgentsException(
"Academy.EnvironmentStep() called recursively. " +
"This might happen if you call EnvironmentStep() from custom code such as " +
"CollectObservations() or OnActionReceived()."
);
}

m_IsStepping = true;

try
using (m_StepRecursionChecker.Start())
{
if (!m_HadFirstReset)
{
Expand Down Expand Up @@ -584,11 +589,6 @@ public void EnvironmentStep()
AgentAct?.Invoke();
}
}
finally
{
// Reset m_IsStepping when we're done (or if an exception occurred).
m_IsStepping = false;
}
}

/// <summary>
Expand Down