fgetline fgets() race handling #920
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update fr.po
Those resources are converted to C and embedded into the final binary during the build. There is no point in also installing them to $(PREFIX)/share/doc/goaccess/
Packaging improvements
…t with when to honor fgets() EOF/EAGAIN
…/EAGAIN.
1) Add getline_{timeout,min_read} defaults to global 'conf'
2) Fixup fgetline() nanosleep to use SLEEP macro, that can be
overridden during compile time if needed.
…ld only be built if WITH_GETLINE is in play.
fgetline() is only called from a wrapped
>> #ifdef WITH_GETLINE
>> read_lines() {
>> ==> fgetline()
>> }
>> #endif
TerraTech
force-pushed
the
fgetlineTimeout
branch
from
6e80a86 to
c6b9bb1
Compare
|
Thanks. I haven't been able to merged this as I'm trying to keep up with some bugs/requests that I plan to squeeze in the next release. I still want to run some tests on my side before merging this commit. |
allinurl
force-pushed
the
master
branch
2 times, most recently
from
0550432 to
b1332f5
Compare
allinurl
force-pushed
the
master
branch
5 times, most recently
from
3d5333b to
6c68a6e
Compare
allinurl
force-pushed
the
master
branch
7 times, most recently
from
31e2ac1 to
c6199f6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch is to handle a fgets() + EOF/EAGAIN race in fgetline(), as well as adding two new switches to tweak its behavior.
I stumbled on this when I changed my log feeder to do something like the following:
{ printf "<custom crafted first log line>\n"; ssh remote gunzip -c logfile; } | pv | goaccess ....The delay would sometimes cause the fgets() to go in a read loop and never exit in the face of either an EOF or EAGAIN.
The
--getline-min-readspecifies the minimum number of lines that must be read before it will quickly exit the read loop on an EOF/EAGAIN. This covers the case where the logs being fed is a combination of 'fast' (printf) and 'slow' feeds where the slow feed incurs a remote ssh connection delay. This covers how EOF/EAGAIN is handled as it should honor it after log lines have been fed for it, but be very conservative and wait for the log stream to start. Non-blocking stream reads can get a bit tricky as the onus is on the coder to cover all the corner cases. (I hope that makes sense).As a belt-n-suspenders approach, it also has a
--getline-timeoutwhich will break out of the loop after specified number of seconds. The default is 10, and has worked well for our use case.Please give it a lookover and let me know what you think. Thanks!