Opened 3 months ago

#126 new defect

Items are stored in the cache even if it's disabled

Reported by: schwa Owned by: schwa
Priority: minor Milestone:
Component: Library Version: 3.4
Keywords: stat cache Cc:

Description

In ftputil.stat, we have the code

    def _stat_results_from_dir(self, path):
        """
        Yield stat results extracted from the directory listing `path`.
        Omit the special entries for the directory itself and its parent
        directory.
        """
        lines = self._host_dir(path)
        # `cache` is the "high-level" `StatCache` object whereas
        # `cache._cache` is the "low-level" `LRUCache` object.
        cache = self._lstat_cache
        # Auto-grow cache if the cache up to now can't hold as many
        # entries as there are in the directory `path`.
        if cache._enabled and len(lines) >= cache._cache.size:
            new_size = int(math.ceil(1.1 * len(lines)))
            cache.resize(new_size)
        # Yield stat results from lines.
        for line in lines:
            if self._parser.ignores_line(line):
                continue
            # Although for a `listdir` call we're only interested in
            # the names, use the `time_shift` parameter to store the
            # correct timestamp values in the cache.
            stat_result = self._parser.parse_line(line,
                                                  self._host.time_shift())
            # Skip entries "." and "..".
            if stat_result._st_name in [self._host.curdir, self._host.pardir]:
                continue
            loop_path = self._path.join(path, stat_result._st_name)
            self._lstat_cache[loop_path] = stat_result
            yield stat_result

According to the second to last line, the stat_result is stored in the cache even if it's disabled.

Usually this will be harmless regardless of whether a user would expect it. The only downside is that if there are many cache assignments like that, the cache won't be expanded because of the cache._enabled test above and the cache logic will remove and replace older cache entries all the time. That said, if the user disabled the stat cache with FTPHost.stat_cache.disable() (the cache is enabled by default), we have much more serious performance problems because the cache isn't used and there will be many roundtrips to the FTP server because of this.

Due to this tradeoff, I think this is a very minor issue. Still, the fix should be very easy by adding an if statement.

Change History (0)

Note: See TracTickets for help on using tickets.