fix(in_tail): fix variable name bugs causing position file leak and NameError#5441
fix(in_tail): fix variable name bugs causing position file leak and NameError#5441magic-peach wants to merge 1 commit into
Conversation
…ameError Two variable name bugs in the tail input plugin: 1. Line 650: @follow_inode (singular) should be @follow_inodes (plural). Ruby silently returns nil for undefined instance variables, so the condition for cleaning up position file entries during rotation silently degenerates, causing unbounded growth of stale entries. 2. Line 709: tail_watcher should be tw (the method parameter). This causes a NameError crash when emit_unmatched_lines is enabled with path_key in multiline mode. Signed-off-by: Akanksha Trehun <akankshatrehun@gmail.com>
Watson1978
left a comment
Watson1978
left a comment
There was a problem hiding this comment.
Thanks for the PR! Two quick things.
First, could you update the description to follow our pull request template? It helps with triage and release notes. Your current summary works well under "What this PR does / why we need it"; it's mainly about adding the remaining headings:
**Which issue(s) this PR fixes**:
Fixes #
**What this PR does / why we need it**:
**Docs Changes**:
**Release Note**:
Second,
the line 754 change, however, is inverted — the enclosing method is convert_line_to_event(line, es, tail_watcher), so tw is not in scope and tw.path raises NameError. master was already correct there, so that line should stay tail_watcher.path. Could you drop the line 754 hunk and keep only the 650 fix?
| if @emit_unmatched_lines | ||
| record = {'unmatched_line' => line} | ||
| record[@path_key] ||= tail_watcher.path unless @path_key.nil? | ||
| record[@path_key] ||= tw.path unless @path_key.nil? |
There was a problem hiding this comment.
This change looks inverted. In convert_line_to_event(line, es, tail_watcher) the parameter is named tail_watcher, and tw is not in scope here (the block parameters are |time, record|). So tw.path raises NameError: undefined local variable or method 'tw' whenever this branch runs (emit_unmatched_lines enabled with path_key).
2 participants
Summary
Two variable name bugs in the tail input plugin (
lib/fluent/plugin/in_tail.rb):1. Wrong instance variable:
@follow_inodevs@follow_inodes(line 650)The condition references
@follow_inode(singular), but the instance variable is defined as@follow_inodes(plural) on line 892. In Ruby, referencing an undefined instance variable silently returnsnilrather than raising an error.When
@follow_inodesis enabled, the position file entry for unwatched tail watchers should always be cleaned up. But because@follow_inodeevaluates tonil, the condition degenerates to(!@tails[tw.path]), so the entry is only unwatched if the path is not currently in@tails. During rotation, when the path IS still present, the position file entry leaks and is never cleaned up, causing unbounded growth with stale entries.2. Undefined local variable
tail_watcher(line 709)In
flush_buffer, the parameter is namedtw, but line 709 referencestail_watcher— an undefined local variable. Every other usage in this method (lines 700, 704, 711, 717) correctly usestw.This causes a
NameError: undefined local variable or method 'tail_watcher'at runtime whenemit_unmatched_linesis enabled withpath_keyin multiline mode, crashing the tail watcher's IOHandler and potentially losing log data.