Skip to content

Commit

Permalink
Use std::shared/unique_ptr to avoid memory leaks
Browse files Browse the repository at this point in the history
  • Loading branch information
nschlia committed Oct 1, 2024
1 parent e029bd9 commit efd6821
Show file tree
Hide file tree
Showing 19 changed files with 350 additions and 295 deletions.
27 changes: 12 additions & 15 deletions src/buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ bool Buffer::init(bool erase_cache)

for (uint32_t segment_no = 1; segment_no <= virtualfile()->get_segment_count(); segment_no++)
{
make_cachefile_name(m_ci[segment_no - 1].m_cachefile, filename() + "." + make_filename(segment_no, params.current_format(virtualfile())->fileext()), params.current_format(virtualfile())->fileext(), false);
make_cachefile_name(&m_ci[segment_no - 1].m_cachefile, filename() + "." + make_filename(segment_no, params.current_format(virtualfile())->fileext()), params.current_format(virtualfile())->fileext(), false);
}
}
else
Expand All @@ -213,19 +213,19 @@ bool Buffer::init(bool erase_cache)
// All other formats: create just a single segment.
m_ci.resize(1);

make_cachefile_name(m_ci[0].m_cachefile, filename(), params.current_format(virtualfile())->fileext(), false);
make_cachefile_name(&m_ci[0].m_cachefile, filename(), params.current_format(virtualfile())->fileext(), false);
if ((virtualfile()->m_flags & VIRTUALFLAG_FRAME))
{
// Create extra index cash for frame sets only
make_cachefile_name(m_ci[0].m_cachefile_idx, filename(), params.current_format(virtualfile())->fileext(), true);
make_cachefile_name(&m_ci[0].m_cachefile_idx, filename(), params.current_format(virtualfile())->fileext(), true);
}
}

// Set current segment
m_cur_ci = &m_ci[0];

// Create the path to the cache file. All paths are the same, so this is required only once.
char *cachefiletmp = new_strdup(m_ci[0].m_cachefile);
std::shared_ptr<char[]> cachefiletmp = new_strdup(m_ci[0].m_cachefile);

if (cachefiletmp == nullptr)
{
Expand All @@ -234,16 +234,13 @@ bool Buffer::init(bool erase_cache)
throw false;
}

if (mktree(dirname(cachefiletmp), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH) && errno != EEXIST)
if (mktree(dirname(cachefiletmp.get()), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH) && errno != EEXIST)
{
Logging::error(m_ci[0].m_cachefile, "Error creating cache directory: (%1) %2", errno, strerror(errno));
delete [] cachefiletmp;
throw false;
}
errno = 0; // reset EEXIST, error can safely be ignored here

delete [] cachefiletmp;

#if __cplusplus >= 202002L
// C++20 (and later) code
for (uint32_t index = 0; CACHEINFO & ci : m_ci)
Expand Down Expand Up @@ -975,24 +972,24 @@ const std::string & Buffer::cachefile(uint32_t segment_no) const
return ci->m_cachefile;
}

const std::string & Buffer::make_cachefile_name(std::string & cachefile, const std::string & filename, const std::string & fileext, bool is_idx)
const std::string & Buffer::make_cachefile_name(std::string * cachefile, const std::string & filename, const std::string & fileext, bool is_idx)
{
transcoder_cache_path(cachefile);

cachefile += params.m_mountpath;
cachefile += filename;
*cachefile += params.m_mountpath;
*cachefile += filename;

if (is_idx)
{
cachefile += ".idx.";
*cachefile += ".idx.";
}
else
{
cachefile += ".cache.";
*cachefile += ".cache.";
}
cachefile += fileext;
*cachefile += fileext;

return cachefile;
return *cachefile;
}

bool Buffer::remove_file(const std::string & filename)
Expand Down
2 changes: 1 addition & 1 deletion src/buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ class Buffer : public FileIO
* @param[in] is_idx - If true, create an index file; otherwise, create a cache.
* @return Returns the name of the cache/index file.
*/
static const std::string & make_cachefile_name(std::string &cachefile, const std::string & filename, const std::string &fileext, bool is_idx);
static const std::string & make_cachefile_name(std::string *cachefile, const std::string & filename, const std::string &fileext, bool is_idx);
/**
* @brief Remove (unlink) the file.
* @param[in] filename - Name of the file to remove.
Expand Down
349 changes: 185 additions & 164 deletions src/cache.cc

Large diffs are not rendered by default.

80 changes: 65 additions & 15 deletions src/cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
#include "buffer.h"

#include <map>
#include <sqlite3.h>
#include <memory>

#define DB_BASE_VERSION_MAJOR 1 /**< @brief The oldest database version major (Release < 1.95) */
#define DB_BASE_VERSION_MINOR 0 /**< @brief The oldest database version minor (Release < 1.95) */
Expand All @@ -48,6 +48,9 @@
#define DB_MIN_VERSION_MAJOR 1 /**< @brief Required database version major (required 1.95) */
#define DB_MIN_VERSION_MINOR 97 /**< @brief Required database version minor (required 1.95) */

typedef struct sqlite3 sqlite3; /**< @brief Forward declaration of sqlite3 handle */
typedef struct sqlite3_stmt sqlite3_stmt; /**< @brief Forward declaration of sqlite3 statement handle */

/**
* @brief RESULTCODE of transcoding operation
*/
Expand Down Expand Up @@ -131,6 +134,62 @@ class Cache

friend class Cache_Entry;

/**
* @brief The sqlite_t class
* Wrapper for sqlite3 struct to make use of std::shared_ptr
*/
class sqlite_t
{
public:
/**
* @brief Construct #sqlite_t object
* @param[in] filename - Database filename (UTF-8)
* @param[in] flags - Flags
* @param[in] zVfs - Name of VFS module to use
*/
explicit sqlite_t(const std::string & filename, int flags, const char *zVfs = nullptr);
/**
* @brief Free #sqlite_t object
*/
virtual ~sqlite_t();

/**
* @brief Return code of last Sqlite operation
* @return Returns result code of last Sqlite operation
*/
int ret() const { return m_ret; };

#ifdef HAVE_SQLITE_CACHEFLUSH
/**
* @brief Flush cache index to disk.
* @return Returns true on success; false on error.
*/
bool flush_index();
#endif // HAVE_SQLITE_CACHEFLUSH

/**
* @brief operator sqlite_t *
* Default return operator.
* @return Returns sqlite_t handle. May be nullptr if invalid.
*/
operator sqlite3*() { return m_db_handle; };

/**
* @brief Get current database file name
* @return Returns current database file name
*/
const std::string & filename() const { return m_filename; };

protected:
int m_ret; /**< @brief Return code of last SQL operation */
std::string m_filename; /**< @brief Name of SQLite cache index database */
sqlite3* m_db_handle; /**< @brief SQLite handle of cache index database */
public:
sqlite3_stmt * m_select_stmt; /**< @brief Prepared select statement */
sqlite3_stmt * m_insert_stmt; /**< @brief Prepared insert statement */
sqlite3_stmt * m_delete_stmt; /**< @brief Prepared delete statement */
};

public:
/**
* @brief Construct #Cache object.
Expand Down Expand Up @@ -164,13 +223,6 @@ class Cache
* @return Returns true on success; false on error.
*/
bool load_index();
#ifdef HAVE_SQLITE_CACHEFLUSH
/**
* @brief Flush cache index to disk.
* @return Returns true on success; false on error.
*/
bool flush_index();
#endif // HAVE_SQLITE_CACHEFLUSH
/**
* @brief Run disk maintenance.
*
Expand Down Expand Up @@ -323,13 +375,11 @@ class Cache
static const TABLE_DEF m_table_version; /**< @brief Definition and indexes of table "version" */
static const TABLECOLUMNS_VEC m_columns_version; /**< @brief Columns of table "version" */

std::recursive_mutex m_mutex; /**< @brief Access mutex */
std::string m_cacheidx_file; /**< @brief Name of SQLite cache index database */
sqlite3* m_cacheidx_db; /**< @brief SQLite handle of cache index database */
sqlite3_stmt * m_cacheidx_select_stmt; /**< @brief Prepared select statement */
sqlite3_stmt * m_cacheidx_insert_stmt; /**< @brief Prepared insert statement */
sqlite3_stmt * m_cacheidx_delete_stmt; /**< @brief Prepared delete statement */
cache_t m_cache; /**< @brief Cache file (memory mapped file) */
std::recursive_mutex m_mutex; /**< @brief Access mutex */

std::unique_ptr<sqlite_t> m_cacheidx_db; /**< @brief SQLite handle of cache index database */

cache_t m_cache; /**< @brief Cache file (memory mapped file) */
};

#endif
4 changes: 1 addition & 3 deletions src/cache_entry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ Cache_Entry::Cache_Entry(Cache *owner, LPVIRTUALFILE virtualfile)
m_cache_info.m_desttype[0] = '\0';
strncat(m_cache_info.m_desttype.data(), params.current_format(virtualfile)->desttype().c_str(), m_cache_info.m_desttype.size() - 1);

m_buffer = new(std::nothrow) Buffer;
m_buffer = std::make_unique<Buffer>();

if (m_buffer != nullptr)
{
Expand All @@ -66,8 +66,6 @@ Cache_Entry::~Cache_Entry()
{
std::unique_lock<std::recursive_mutex> lock_active_mutex(m_active_mutex);

delete m_buffer;

unlock();

Logging::trace(filename(), "Deleted buffer.");
Expand Down
2 changes: 1 addition & 1 deletion src/cache_entry.h
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ class Cache_Entry
LPVIRTUALFILE m_virtualfile; /**< @brief Underlying virtual file object */

public:
Buffer * m_buffer; /**< @brief Buffer object */
std::unique_ptr<Buffer> m_buffer; /**< @brief Buffer object */
std::atomic_bool m_is_decoding; /**< @brief true while file is decoding */
std::recursive_mutex m_active_mutex; /**< @brief Mutex while thread is active */
std::recursive_mutex m_restart_mutex; /**< @brief Mutex while thread is restarted */
Expand Down
11 changes: 4 additions & 7 deletions src/ffmpeg_transcoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ void FFmpeg_Transcoder::StreamRef::reset()
#pragma GCC diagnostic ignored "-Wmissing-field-initializers"
FFmpeg_Transcoder::FFmpeg_Transcoder()
: m_fileio(nullptr)
, m_close_fileio(true)
, m_last_seek_frame_no(0)
, m_have_seeked(false)
, m_skip_next_frame(false)
Expand Down Expand Up @@ -292,7 +291,7 @@ bool FFmpeg_Transcoder::is_open() const
return (m_in.m_format_ctx != nullptr);
}

int FFmpeg_Transcoder::open_input_file(LPVIRTUALFILE virtualfile, FileIO *fio)
int FFmpeg_Transcoder::open_input_file(LPVIRTUALFILE virtualfile, std::shared_ptr<FileIO> fio)
{
AVDictionary * opt = nullptr;
int ret;
Expand Down Expand Up @@ -349,13 +348,11 @@ int FFmpeg_Transcoder::open_input_file(LPVIRTUALFILE virtualfile, FileIO *fio)
{
// Open new file io
m_fileio = FileIO::alloc(m_virtualfile->m_type);
m_close_fileio = true; // do not close and delete
}
else
{
// Use already open file io
m_fileio = fio;
m_close_fileio = false; // must not close or delete
}

if (m_fileio == nullptr)
Expand Down Expand Up @@ -391,7 +388,7 @@ int FFmpeg_Transcoder::open_input_file(LPVIRTUALFILE virtualfile, FileIO *fio)
iobuffer,
static_cast<int>(m_fileio->bufsize()),
0,
static_cast<void *>(m_fileio),
static_cast<void *>(m_fileio.get()),
input_read,
nullptr, // input_write
seek); // input_seek
Expand Down Expand Up @@ -6449,11 +6446,11 @@ bool FFmpeg_Transcoder::close_input_file()
{
closed = true;

if (m_close_fileio && m_fileio != nullptr)
if (m_fileio.use_count() <= 1 && m_fileio != nullptr)
{
m_fileio->closeio();
save_delete(&m_fileio);
}
m_fileio.reset();

if (m_in.m_format_ctx->pb != nullptr)
{
Expand Down
5 changes: 2 additions & 3 deletions src/ffmpeg_transcoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ class FFmpeg_Transcoder : public FFmpeg_Base, FFmpeg_Profiles
*
* @return On success, returns 0; on error, a negative AVERROR value.
*/
int open_input_file(LPVIRTUALFILE virtualfile, FileIO * fio = nullptr);
int open_input_file(LPVIRTUALFILE virtualfile, std::shared_ptr<FileIO> fio = nullptr);
/**
* @brief Open output file. Data will actually be written to buffer and copied by FUSE when accessed.
* @param[in] buffer - Cache buffer to be written.
Expand Down Expand Up @@ -1226,8 +1226,7 @@ class FFmpeg_Transcoder : public FFmpeg_Base, FFmpeg_Profiles
int foreach_subtitle_file(const std::string& search_path, const std::regex& regex, int depth, const std::function<int (const std::string &, const std::optional<std::string> &)> & f);

private:
FileIO * m_fileio; /**< @brief FileIO object of input file */
bool m_close_fileio; /**< @brief If we own the FileIO object, we may close it in the end. */
std::shared_ptr<FileIO> m_fileio; /**< @brief FileIO object of input file */
time_t m_mtime; /**< @brief Modified time of input file */
std::recursive_mutex m_seek_to_fifo_mutex; /**< @brief Access mutex for seek FIFO */
std::queue<uint32_t> m_seek_to_fifo; /**< @brief Stack of seek requests. Will be processed FIFO */
Expand Down
Loading

0 comments on commit efd6821

Please sign in to comment.