Skip to content

Small threading fix, delay to main thread#19

Merged
mcpiroman merged 2 commits into
mcpiroman:masterfrom
rogerbarton:threading-fix
May 1, 2020
Merged

Small threading fix, delay to main thread#19
mcpiroman merged 2 commits into
mcpiroman:masterfrom
rogerbarton:threading-fix

Conversation

@rogerbarton
Copy link
Copy Markdown
Contributor

Previously when calling a function on a worker thread and encountering a load error, setting the pause state would trigger a exception as we use the Unity API on a worker thread.

This is only for Lazy mode, where the thread safety is not yet fully implemented. But it still fixes this issue.

Previously when calling a function on a worker thread and encountering a load error, pausing would trigger a exception as we use the Unity API on a worker thread. 

This is only for Lazy mode, where the thread safety is not yet fully implemented.
@mcpiroman
Copy link
Copy Markdown
Owner

So maybe just extract the dispatching logic to sth like:

void DispatchOnMainThread(Action action)
{
  if(Thread.CurrentThread.ManagedThreadId == _unityMainThreadId)
     action();
  else
     DllManipulatorScript.MainThreadTriggerQueue.Enqueue(action);
}

and use it both here and on previous PR

@rogerbarton
Copy link
Copy Markdown
Contributor Author

Yes agreed. However, in the logic of the previous PR there's also an additional bool useMainThreadQueue:

if (useMainThreadQueue && Thread.CurrentThread.ManagedThreadId != _unityMainThreadId)
DllManipulatorScript.MainThreadTriggerQueue.Enqueue(() => methodInfo.Invoke(null, args));
else
methodInfo.Invoke(null, args);

I've left it as is in the previous PR (seeing as its only one occurrence) and changed it for these diffs. Or do you see an nice way of doing this?

@mcpiroman
Copy link
Copy Markdown
Owner

Yup I forgot about that flag there. You could do something like this but I don't see much sense in doing so

if(useMainThreadQueue)
   DispatchOnMainThread(() => methodInfo.Invoke(null, args));
else
  methodInfo.Invoke(null, args)

so yes, leave this as is, whatever

@mcpiroman mcpiroman merged commit dd52d89 into mcpiroman:master May 1, 2020
@rogerbarton rogerbarton deleted the threading-fix branch May 1, 2020 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants