Skip to content
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.

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

Add ConfigureAwait(false) to a couple of places to avoid potential deadlock #118

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

sdiaman1
Copy link

@sdiaman1 sdiaman1 commented May 13, 2022

Minimal, reproducible example:

// Configure the logging.
LogManagerFactory.DefaultConfiguration = new LoggingConfiguration();
StreamingFileTarget streamingFileTarget = new StreamingFileTarget();
LogManagerFactory.DefaultConfiguration.AddTarget(LogLevel.Trace, LogLevel.Fatal, streamingFileTarget);
LogManagerFactory.DefaultConfiguration.IsEnabled = true;

// Log a message.
ILogger log = LogManagerFactory.DefaultLogManager.GetLogger("logger");
log.Info("This is an info message");

// Get the compressed logs...this code deadlocks with the above log message.
Stream compressedLogs = null;
Task.Run(async () => compressedLogs = await LogManagerFactory.DefaultLogManager.GetCompressedLogs()).Wait();

I tried most (all?) of potential solutions at https://stackoverflow.com/questions/9343594/how-to-call-asynchronous-method-from-synchronous-method-in-c, but nothing worked for me. Except for adding ConfigureAwait(false) to a couple of places in this library....The changes in this pull request.

I think I'm trying to do basically the same thing as #117.

I don't know if ConfigureAwait(false) should also be added to additional places in this library. Also, I only tested this PR's changes on Windows, so there may be additional changes that are needed to get this to work on Android and iOS, and also maybe for other scenarios that we didn't test for on Windows.

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.

1 participant