Cleaned up changes to session manager.

Persist resume data on the first shutdown pass; consume futures with one error policy.

doc/rtorrent.rc

@@ -45,13 +45,6 @@
 #
 #session.path.set = ./session
 
-# Umask for files written to the session directory, default 0077 (files
-# are mode 0600). Use 0007 for 0660 or 0027 for 0640. The owner must keep
-# read/write access. Session files contain tracker announce URLs
-# (potentially passkeys), so loosening permissions is a user choice.
-#
-#session.umask.set = 0077
-
 # Watch a directory for new torrents, and stop those that have been
 # deleted. Use directory.watch.ready for network shares or other watch
 # directories where files may be copied or written directly into place.
@@ -59,11 +52,11 @@
 # watch directory.
 #
 #directory.watch.ready = ./watch, load.start
-#schedule = watch_directory,5,5,load.start=./watch/*.torrent
+#schedule2 = watch_directory,5,5,load.start=./watch/*.torrent
 
 # Close torrents when disk-space is low.
 #
-#schedule = low_diskspace,5,60,close_low_diskspace=100M
+#schedule2 = low_diskspace,5,60,close_low_diskspace=100M
 
 # The IP address reported to the tracker.
 #
@@ -107,8 +100,8 @@
 
 # Alternative calls to bind and IP that should handle dynamic IP's.
 #
-#schedule = ip_tick,0,1800,ip=rakshasa
+#schedule2 = ip_tick,0,1800,ip=rakshasa
-#schedule = bind_tick,0,1800,bind=rakshasa
+#schedule2 = bind_tick,0,1800,bind=rakshasa
 
 # Encryption options, set to none (default) or any combination of the following:
 # allow_incoming, try_outgoing, require, require_RC4, enable_retry, prefer_plaintext
@@ -152,4 +145,4 @@
 #
 #network.scgi.open_port = "127.0.0.1:5000"
 #network.scgi.open_local = (cat,(session.path),/rpc.sock)
-#schedule = socket_chmod, 0, 0, "execute.nothrow=chmod,770,(cat,(session.path),/rpc.sock)"
+#schedule2 = socket_chmod, 0, 0, "execute.nothrow=chmod,770,(cat,(session.path),/rpc.sock)"

doc/rtorrent.rc-example

@@ -30,6 +30,7 @@ network.listen.port.random.set = no
 # (conservative settings for 'private' trackers, change for 'public')
 dht.mode.set = disable
 protocol.pex.set = no
+trackers.use_udp.set = no
 
 # Peer settings
 throttle.max_uploads.set = 100
@@ -41,22 +42,14 @@ throttle.min_peers.seed.set = 30
 throttle.max_peers.seed.set = 80
 trackers.numwant.set = 80
 
-# Select encryption for handshake and connection:
+protocol.encryption.set = allow_incoming,try_outgoing,enable_retry
-#
-# protocol.encryption.set = {deny,allow,prefer,require}
-# protocol.encryption.set = handshake_{deny,allow,prefer,require}, stream_{deny,allow,prefer,require}
-#
-#protocol.encryption.set = deny
-#protocol.encryption.set = require
-#protocol.encryption.set = handshake_prefer, stream_deny
-#protocol.encryption.set = handshake_require, stream_prefer
 
 # Limits for file handle resources, this is optimized for
 # an `ulimit` of 1024 (a common default). You MUST leave
 # a ceiling of handles reserved for rTorrent's internal needs!
-
+network.http.max_open.set = 50
-# Deprecated, see: https://github.com/rakshasa/rtorrent/wiki/Socket-Manager-and-Resource-Allocation
+network.max_open_files.set = 600
-
+network.max_open_sockets.set = 300
 
 # Memory resource usage (increase if you have a large number of items loaded,
 # and/or the available resources to spend)
@@ -72,10 +65,11 @@ execute.nothrow = sh, -c, (cat, "echo >",\
     (session.path), "rtorrent.pid", " ", (system.pid))
 
 # Other operational settings (check & adapt)
+encoding.add = utf8
 system.umask.set = 0027
 system.cwd.set = (directory.default)
 network.http.dns_cache_timeout.set = 25
-schedule = monitor_diskspace, 15, 60, ((close_low_diskspace, 1000M))
+schedule2 = monitor_diskspace, 15, 60, ((close_low_diskspace, 1000M))
 ##pieces.hash.on_completion.set = no
 ##view.sort_current = seeding, greater=d.ratio=
 ##keys.layout.set = qwerty
@@ -94,8 +88,8 @@ method.insert = d.data_path, simple,\
 method.insert = d.session_file, simple, "cat=(session.path), (d.hash), .torrent"
 
 # Watch directories (add more as you like, but use unique schedule names)
-schedule = watch_start, 10, 10, ((load.start_verbose, (cat, (cfg.watch), "start/*.torrent")))
+schedule2 = watch_start, 10, 10, ((load.start_verbose, (cat, (cfg.watch), "start/*.torrent")))
-schedule = watch_load, 11, 10, ((load.verbose, (cat, (cfg.watch), "load/*.torrent")))
+schedule2 = watch_load, 11, 10, ((load.verbose, (cat, (cfg.watch), "load/*.torrent")))
 
 # Run the rTorrent process as a daemon in the background
 # (and control via XMLRPC sockets)

doc/rtorrent.rc.lua-example

@@ -60,6 +60,7 @@ rc.network.listen.port.random = false
 -- (conservative settings for 'private' trackers, change for 'public')
 rc.dht.mode = 'disable'
 rc.protocol.pex = false
+rc.trackers.use_udp = false
 
 -- Peer settings
 rc.throttle.max_uploads = 100
@@ -71,11 +72,14 @@ rc.throttle.min_peers.seed = 30
 rc.throttle.max_peers.seed = 80
 rc.trackers.numwant = 80
 
+rc.protocol.encryption.set('allow_incoming', 'try_outgoing', 'enable_retry')
+
 -- Limits for file handle resources, this is optimized for
 -- an `ulimit` of 1024 (a common default). You MUST leave
 -- a ceiling of handles reserved for rTorrent's internal needs!
-
+rc.network.http.max_open = 50
--- Deprecated, see: https://github.com/rakshasa/rtorrent/wiki/Socket-Manager-and-Resource-Allocation
+rc.network.max_open_files = 600
+rc.network.max_open_sockets = 300
 
 -- Memory resource usage (increase if you have a large number of items loaded,
 -- and/or the available resources to spend)
@@ -93,10 +97,11 @@ rc.execute.nothrow(
 }))
 
 -- Other operational settings (check & adapt)
+rc.encoding.add('utf8')
 rc.system.umask = 0027
 rc.system.cwd = rc.directory.default()
 rc.network.http.dns_cache_timeout = 25
-rc.schedule('monitor_diskspace', '15', '60', 'close_low_diskspace=1000M')
+rc.schedule2('monitor_diskspace', '15', '60', 'close_low_diskspace=1000M')
 --rc.pieces.hash.on_completion = false
 --rc.view.sort_current('seeding', 'greater=d.ratio=')
 --rc.keys.layout = 'qwerty'
@@ -116,8 +121,8 @@ rc.method.insert('d.data_path', 'simple',
 rc.method.insert('d.session_file', 'simple', 'cat=(session.path), (d.hash), .torrent')
 
 -- Watch directories (add more as you like, but use unique schedule names)
-rc.schedule('watch_start', '10', '10', 'load.start_verbose=(cat, (cfg.watch), "start/*.torrent")')
+rc.schedule2('watch_start', '10', '10', 'load.start_verbose=(cat, (cfg.watch), "start/*.torrent")')
-rc.schedule('watch_load', '11', '10', 'load.verbose=(cat, (cfg.watch), "load/*.torrent")')
+rc.schedule2('watch_load', '11', '10', 'load.verbose=(cat, (cfg.watch), "load/*.torrent")')
 
 -- Run the rTorrent process as a daemon in the background
 -- (and control via XMLRPC sockets)

src/command_local.cc

@@ -123,8 +123,6 @@ initialize_command_local() {
   CMD_ANY_STRING_V("session.path.set",                [](auto, auto& str)   { return session_thread::manager()->set_path(str); });
   CMD_ANY         ("session.use_lock",                [](auto, auto)        { return session_thread::manager()->use_lock(); });
   CMD_ANY_VALUE_V ("session.use_lock.set",            [](auto, auto& value) { return session_thread::manager()->set_use_lock(value); });
-  CMD_ANY         ("session.umask",                   [](auto, auto)        { return session_thread::manager()->umask(); });
-  CMD_ANY_VALUE_V ("session.umask.set",               [](auto, auto& value) { return session_thread::manager()->set_umask(value); });
   CMD_VAR_BOOL    ("session.on_completion",           true);
 
   CMD_ANY_V       ("session.save",                    [dList](auto, auto)   { return dList->session_save(); });
@@ -173,7 +171,6 @@ initialize_command_local() {
   rpc::rpc.mark_safe("directory.default.realpath.or_empty");
   rpc::rpc.mark_safe("directory.default.realpath.or_throw");
   rpc::rpc.mark_safe("session.use_lock");
-  rpc::rpc.mark_safe("session.umask");
   rpc::rpc.mark_safe("session.on_completion");
 
   rpc::rpc.mark_safe("pieces.sync.always_safe");

src/control.cc

@@ -10,6 +10,7 @@
 #include <torrent/utils/directory_events.h>
 
 #include "core/dht_manager.h"
+#include "core/download_list.h"
 #include "core/http_queue.h"
 #include "core/manager.h"
 #include "core/view_manager.h"
@@ -137,6 +138,15 @@ Control::handle_shutdown() {
   if (scgi_thread::thread()->is_active())
     scgi_thread::thread()->stop_thread_wait();
 
+  if (!m_shutdown_session_saved) {
+    // Flush the latest state before stopped announces can consume the timeout. A shutdown hook may
+    // already have queued these resume builds.
+    m_core->download_list()->session_save();
+    session_thread::manager()->flush_shutdown_saves();
+
+    m_shutdown_session_saved = true;
+  }
+
   if (!m_shutdown_quick) {
     torrent::runtime::network_manager()->listen_close();
     torrent::runtime::shutdown();

src/control.h

@@ -112,6 +112,8 @@ private:
 
   int                 m_clear_requests_count{};
 
+  bool                m_shutdown_session_saved{};
+
   align_cacheline
 
   std::atomic<bool>   m_shutdown_received{};

src/session/download_storer.cc

@@ -5,7 +5,6 @@
 #include <cerrno>
 #include <fcntl.h>
 #include <fstream>
-#include <sys/stat.h>
 #include <unistd.h>
 #include <torrent/exceptions.h>
 #include <torrent/object.h>
@@ -124,7 +123,7 @@ is_correct_format(const std::string& f) {
 }
 
 void
-save_stream(const std::string& path, bool use_fsyncdisk, mode_t file_mode, const std::stringstream& stream) {
+save_stream(const std::string& path, bool use_fsyncdisk, const std::stringstream& stream) {
   // Remove any leftover temporary file first so that O_EXCL only ever fails on
   // an entry that appeared after the unlink, and O_NOFOLLOW keeps a symlink
   // planted in the session directory from redirecting the write.
@@ -137,12 +136,6 @@ save_stream(const std::string& path, bool use_fsyncdisk, mode_t file_mode, const
   if (fd < 0)
     throw torrent::storage_error("failed to open file for writing : " + path);
 
-  // open() applies the process umask, so set the configured mode explicitly.
-  if (::fchmod(fd, file_mode) == -1) {
-    ::close(fd);
-    throw torrent::storage_error("failed to set file permissions : " + path);
-  }
-
   const auto  data      = stream.view();
   std::size_t remaining = data.size();
   const char* cursor    = data.data();
@@ -203,7 +196,7 @@ sync_directory_of(const std::string& path) {
 } // namespace anonymous
 
 void
-DownloadStorer::save_and_move_streams(const std::string& path, bool use_fsyncdisk, mode_t file_mode,
+DownloadStorer::save_and_move_streams(const std::string& path, bool use_fsyncdisk,
                                        const std::stringstream* torrent_stream,
                                        const std::stringstream* rtorrent_stream,
                                        const std::stringstream* libtorrent_stream) {
@@ -212,10 +205,10 @@ DownloadStorer::save_and_move_streams(const std::string& path, bool use_fsyncdis
   auto rtorrent_path   = path + ".rtorrent";
 
   if (torrent_stream)
-    save_stream(torrent_path + ".new", use_fsyncdisk, file_mode, *torrent_stream);
+    save_stream(torrent_path + ".new", use_fsyncdisk, *torrent_stream);
 
-  save_stream(libtorrent_path + ".new", use_fsyncdisk, file_mode, *libtorrent_stream);
+  save_stream(libtorrent_path + ".new", use_fsyncdisk, *libtorrent_stream);
-  save_stream(rtorrent_path + ".new", use_fsyncdisk, file_mode, *rtorrent_stream);
+  save_stream(rtorrent_path + ".new", use_fsyncdisk, *rtorrent_stream);
 
   if (torrent_stream) {
     if (::rename((torrent_path + ".new").c_str(), torrent_path.c_str()) == -1)

src/session/download_storer.h

@@ -3,7 +3,6 @@
 
 #include <memory>
 #include <sstream>
-#include <sys/types.h>
 #include <string>
 #include <torrent/common.h>
 
@@ -34,7 +33,7 @@ public:
   auto                rtorrent_stream()      { return std::move(m_rtorrent_stream); }
   auto                libtorrent_stream()    { return std::move(m_libtorrent_stream); }
 
-  static void         save_and_move_streams(const std::string& path, bool use_fsyncdisk, mode_t file_mode,
+  static void         save_and_move_streams(const std::string& path, bool use_fsyncdisk,
                                             const std::stringstream* torrent_stream,
                                             const std::stringstream* rtorrent_stream,
                                             const std::stringstream* libtorrent_stream);

src/session/session_manager.cc

@@ -65,22 +65,6 @@ SessionManager::set_use_lock(bool use_lock) {
   m_use_lock = use_lock;
 }
 
-void
-SessionManager::set_umask(int64_t umask) {
-  assert(torrent::this_thread::thread() == torrent::main_thread::thread());
-
-  if (m_freeze_info)
-    throw torrent::input_error("Session umask option cannot be changed after startup.");
-
-  if (umask < 0 || umask > 0777)
-    throw torrent::input_error("Session umask must be in the range 0 to 0777.");
-
-  if ((umask & 0600) != 0)
-    throw torrent::input_error("Session umask must not mask owner read/write permissions (0600).");
-
-  m_umask = umask;
-}
-
 void
 SessionManager::save_resume_download(core::Download* download) {
   assert(torrent::this_thread::thread() == torrent::main_thread::thread());
@@ -395,7 +379,7 @@ SessionManager::process_next_save_request_unsafe() {
         };
 
       try {
-        DownloadStorer::save_and_move_streams(itr->second.path, m_use_fsyncdisk, file_mode(),
+        DownloadStorer::save_and_move_streams(itr->second.path, m_use_fsyncdisk,
                                               itr->second.torrent_stream.get(),
                                               itr->second.rtorrent_stream.get(),
                                               itr->second.libtorrent_stream.get());
@@ -421,42 +405,67 @@ SessionManager::process_finished_saves() {
   if (!m_active)
     throw torrent::internal_error("SessionManager::process_finished_saves() called while not active.");
 
-  for (auto& request : m_finished_saves) {
+  for (auto& request : m_finished_saves)
-    try {
+    consume_finished_save_unsafe(request);
-      request.first.get();
-
-    } catch (torrent::storage_error& e) {
-      LT_LOG("error saving download : storage error :download:%p path:%s : %s", request.second.download, request.second.path.c_str(), e.what());
-
-      if (m_last_storage_error_message + std::chrono::minutes(5) > torrent::this_thread::cached_time()) {
-        m_ignored_storage_error_count++;
-        continue;
-      }
-
-      lt_log_print(torrent::LOG_ERRORS, "Storage errors saving session data for download: ignored:%u : %s", m_ignored_storage_error_count, e.what());
-
-      m_last_storage_error_message = torrent::this_thread::cached_time();
-      m_ignored_storage_error_count = 0;
-
-      continue;
-
-    } catch (torrent::internal_error& e) {
-      LT_LOG("error saving download : internal error : download:%p path:%s : %s", request.second.download, request.second.path.c_str(), e.what());
-      throw;
-
-    } catch (...) {
-      LT_LOG("error saving download : unknown error : download:%p path:%s", request.second.download, request.second.path.c_str());
-      throw;
-    }
-
-    LT_LOG("finished saving download : download:%p path:%s", request.second.download, request.second.path.c_str());
-  }
 
   m_finished_saves.clear();
 
   callback_save_request();
 }
 
+// Returns true when a storage error was suppressed.
+bool
+SessionManager::report_storage_error_unsafe(const std::string& msg) {
+  if (m_last_storage_error_message + std::chrono::minutes(5) > torrent::this_thread::cached_time()) {
+    m_ignored_storage_error_count++;
+    return true;
+  }
+
+  lt_log_print(torrent::LOG_ERRORS, "Storage errors saving session data for download: ignored:%u : %s", m_ignored_storage_error_count, msg.c_str());
+
+  m_last_storage_error_message  = torrent::this_thread::cached_time();
+  m_ignored_storage_error_count = 0;
+
+  return false;
+}
+
+// The one policy for a finished save, on the session thread and in shutdown flushes: storage errors
+// go to report_storage_error_unsafe(), internal and unknown errors propagate.
+//
+// Returns true when a storage error was suppressed.
+bool
+SessionManager::consume_finished_save_unsafe(ProcessingSave& request) {
+  try {
+    request.first.get();
+
+  } catch (torrent::storage_error& e) {
+    LT_LOG("error saving download : storage error :download:%p path:%s : %s", request.second.download, request.second.path.c_str(), e.what());
+    return report_storage_error_unsafe(e.what());
+
+  } catch (torrent::internal_error& e) {
+    LT_LOG("error saving download : internal error : download:%p path:%s : %s", request.second.download, request.second.path.c_str(), e.what());
+    throw;
+
+  } catch (...) {
+    LT_LOG("error saving download : unknown error : download:%p path:%s", request.second.download, request.second.path.c_str());
+    throw;
+  }
+
+  LT_LOG("finished saving download : download:%p path:%s", request.second.download, request.second.path.c_str());
+  return false;
+}
+
+void
+SessionManager::flush_shutdown_saves() {
+  assert(torrent::this_thread::thread() == torrent::main_thread::thread());
+
+  flush_all_pending_builds();
+
+  std::unique_lock<std::mutex> lock(m_mutex);
+
+  flush_all_and_wait_unsafe(lock);
+}
+
 void
 SessionManager::flush_all_and_wait_unsafe(std::unique_lock<std::mutex>& lock) {
   LT_LOG("flushing all pending saves", 0);
@@ -475,11 +484,20 @@ SessionManager::flush_all_and_wait_unsafe(std::unique_lock<std::mutex>& lock) {
   while (!m_processing_saves.empty())
     m_finished_condition.wait(lock);
 
+  unsigned int suppressed = 0;
+
   for (auto& request : m_finished_saves)
-    LT_LOG("finished saving download : download:%p path:%s", request.second.download, request.second.path.c_str());
+    suppressed += consume_finished_save_unsafe(request) ? 1 : 0;
 
   m_finished_saves.clear();
 
+  if (m_ignored_storage_error_count != 0) {
+    lt_log_print(torrent::LOG_ERRORS, "Storage errors flushing session data: ignored:%u previous:%u",
+                 suppressed, m_ignored_storage_error_count - suppressed);
+
+    m_ignored_storage_error_count = 0;
+  }
+
   LT_LOG("flushed all pending saves", 0);
 }
 

src/session/session_manager.h

@@ -6,8 +6,6 @@
 #include <deque>
 #include <future>
 #include <list>
-#include <cstdint>
-#include <sys/types.h>
 #include <memory>
 #include <mutex>
 #include <sstream>
@@ -61,10 +59,6 @@ public:
   bool                use_lock() const;
   void                set_use_lock(bool use_lock);
 
-  int                 umask() const;
-  void                set_umask(int64_t umask);
-  mode_t              file_mode() const;
-
   void                save_full_download(core::Download* download);
   void                save_resume_download(core::Download* download);
   void                remove_download(core::Download* download);
@@ -79,8 +73,10 @@ protected:
   void                cleanup();
 
   void                flush_all_pending_builds();
+  void                flush_shutdown_saves();
 
 private:
+
   void                callback_pending_builds();
   void                callback_save_request();
   void                callback_finished_saves();
@@ -98,6 +94,9 @@ private:
 
   using ProcessingSave = std::pair<std::future<void>, SaveRequest>;
 
+  bool                consume_finished_save_unsafe(ProcessingSave& request);
+  bool                report_storage_error_unsafe(const std::string& msg);
+
   torrent::system::Thread*     m_thread;
   torrent::system::callback_id m_callback_id;
 
@@ -105,7 +104,6 @@ private:
   std::string                  m_path;
   bool                         m_use_fsyncdisk{true};
   bool                         m_use_lock{true};
-  int                          m_umask{0077};
 
   align_cacheline
 
@@ -140,8 +138,6 @@ inline bool        SessionManager::is_used() const            { return !m_path.e
 inline std::string SessionManager::path() const               { return m_path; }
 inline bool        SessionManager::use_fsyncdisk() const      { return m_use_fsyncdisk; }
 inline bool        SessionManager::use_lock() const           { return m_use_lock; }
-inline int         SessionManager::umask() const              { return m_umask; }
-inline mode_t      SessionManager::file_mode() const          { return 0666 & ~m_umask; }
 inline void        SessionManager::flush_all_pending_builds() { process_pending_builds(true); }
 
 } // namespace session

test/src/test_session_commit.cc

@@ -73,7 +73,7 @@ TestSessionCommit::commit_and_verify(bool use_fsyncdisk) {
   std::stringstream rtorrent_stream("rtorrent-data");
   std::stringstream libtorrent_stream("libtorrent-data");
 
-  session::DownloadStorer::save_and_move_streams(path, use_fsyncdisk, 0600, &torrent_stream, &rtorrent_stream, &libtorrent_stream);
+  session::DownloadStorer::save_and_move_streams(path, use_fsyncdisk, &torrent_stream, &rtorrent_stream, &libtorrent_stream);
 
   CPPUNIT_ASSERT_EQUAL(std::string("torrent-data"), read_file(path));
   CPPUNIT_ASSERT_EQUAL(std::string("rtorrent-data"), read_file(path + ".rtorrent"));

test/src/test_session_storer.cc

@@ -39,12 +39,12 @@ read_file(const std::string& path) {
 }
 
 void
-save_session_files(const std::string& path, mode_t file_mode = 0600) {
+save_session_files(const std::string& path) {
   std::stringstream torrent_stream("torrent-data");
   std::stringstream rtorrent_stream("rtorrent-data");
   std::stringstream libtorrent_stream("libtorrent-data");
 
-  session::DownloadStorer::save_and_move_streams(path, false, file_mode, &torrent_stream, &rtorrent_stream, &libtorrent_stream);
+  session::DownloadStorer::save_and_move_streams(path, false, &torrent_stream, &rtorrent_stream, &libtorrent_stream);
 }
 
 unsigned int
@@ -130,21 +130,6 @@ TestSessionStorer::test_saved_files_are_owner_only() {
   CPPUNIT_ASSERT_EQUAL(0600u, permissions_of(path + ".libtorrent_resume"));
 }
 
-void
-TestSessionStorer::test_saved_files_use_configured_mode() {
-  for (mode_t mode : {0660, 0640}) {
-    auto path       = m_session_dir + "/" + entry_name;
-    auto prev_umask = ::umask(0);
-
-    save_session_files(path, mode);
-    ::umask(prev_umask);
-
-    CPPUNIT_ASSERT_EQUAL((unsigned int)mode, permissions_of(path));
-    CPPUNIT_ASSERT_EQUAL((unsigned int)mode, permissions_of(path + ".rtorrent"));
-    CPPUNIT_ASSERT_EQUAL((unsigned int)mode, permissions_of(path + ".libtorrent_resume"));
-  }
-}
-
 // Entries listed for loading must report their real type so that a symlink in
 // the session directory is skipped instead of loaded.
 void

test/src/test_session_storer.h

@@ -7,7 +7,6 @@ class TestSessionStorer : public test_fixture {
 
   CPPUNIT_TEST(test_temp_file_symlink_is_not_followed);
   CPPUNIT_TEST(test_saved_files_are_owner_only);
-  CPPUNIT_TEST(test_saved_files_use_configured_mode);
   CPPUNIT_TEST(test_symlinked_entry_is_not_a_file);
 
   CPPUNIT_TEST_SUITE_END();
@@ -18,7 +17,6 @@ public:
 
   void test_temp_file_symlink_is_not_followed();
   void test_saved_files_are_owner_only();
-  void test_saved_files_use_configured_mode();
   void test_symlinked_entry_is_not_a_file();
 
 private: