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.

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

Jump to bottom

Stop FlushAsync from preventing application exit #227

Merged
merged 5 commits into from
Jun 16, 2020
Merged

Stop FlushAsync from preventing application exit #227

merged 5 commits into from
Jun 16, 2020

Conversation

snakefoot
Copy link
Contributor

@snakefoot snakefoot commented May 13, 2020 •
edited by luigiberrettini
Loading

Closes #226

@snakefoot snakefoot mentioned this pull request May 13, 2020
Closed
@iangregory
Copy link

I've tested this and can confirm it resolves the issue I reported - thanks.

@snakefoot
Copy link
Contributor Author

@luigiberrettini Any chance of having this merged along with #217, since actual bugs?

@luigiberrettini luigiberrettini added this to the 6.0.1 milestone Jun 14, 2020
@luigiberrettini
Copy link
Owner

A different strategy has to be chosen to avoid regression and solve the bug reported in this issue

@snakefoot
Copy link
Contributor Author

snakefoot commented Jun 15, 2020 •
edited
Loading

The SysLog-target is the one with the buggy-FlushAync-method. If the syslog-target behaved as intended, then you will notice that LogManager.Flush()-method will nicely wait for the FlushAsync to call the provided asyncContinuation.

This PR is the correct way of implementing the FlushAsync-method. And it also fixes the currently broken contract between InitializeTarget and CloseTarget.

@luigiberrettini
Copy link
Owner

I will perform some tests to see if everything works and see if I want to proceed this way, thanks for contributing!

@luigiberrettini
Copy link
Owner

Both #147 and #226 are solved without using a Wait and just adopting NLog conventions properly as @snakefoot suggested while my brain was rejecting the idea

@luigiberrettini luigiberrettini changed the title Avoid blocking forever in FlushAsync() and preventing CloseTarget() Stop Flush from preventing application exit Jun 16, 2020
@luigiberrettini luigiberrettini changed the title Stop Flush from preventing application exit Stop FlushAsync from preventing application exit Jun 16, 2020
@luigiberrettini luigiberrettini merged commit 53a8d9c into luigiberrettini:master Jun 16, 2020
luigiberrettini added a commit to snakefoot/NLog.Targets.Syslog that referenced this pull request Jun 16, 2020
@luigiberrettini
Merge back luigiberrettini#227
3e48b0c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luigiberrettini luigiberrettini luigiberrettini left review comments

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
6.0.1
Development

Successfully merging this pull request may close these issues.

Flush prevents app exit when the server is down
3 participants
@snakefoot @iangregory @luigiberrettini