Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix pre-allocation when changing priority #7738

Merged
merged 2 commits into from
Sep 30, 2024
Merged

fix pre-allocation when changing priority #7738

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa4576c
based on @HanabishiRecca 's research and fix. set m_file_priority ear…
arvidn Sep 29, 2024
74c7c8b
add test
arvidn Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ChangeLog
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@

2.0.11 not released

* fix file pre-allocation when changing file priority (HanabishiRecca)
* fix uTP issue where closing the connection could corrupt the payload
* apply DSCP/TOS to sockets before initiating the TCP connection
* assume copy_file_range() exists on linux (unless old glibc)
Expand Down
30 changes: 15 additions & 15 deletions src/mmap_storage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ POSSIBILITY OF SUCH DAMAGE.
#include "libtorrent/aux_/drive_info.hpp"
#include "libtorrent/stat_cache.hpp"
#include "libtorrent/hex.hpp" // to_hex
#include "libtorrent/aux_/scope_end.hpp"

#if TORRENT_HAVE_MMAP || TORRENT_HAVE_MAP_VIEW_OF_FILE

Expand Down Expand Up @@ -161,15 +162,22 @@ error_code translate_error(std::error_code const& err, bool const write)

download_priority_t const old_prio = m_file_priority[i];
download_priority_t new_prio = prio[i];

m_file_priority[i] = new_prio;
Copy link
Contributor

@HanabishiRecca HanabishiRecca Sep 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I'm not a C++ guy, but this looks sketchy to me. Also considering that new_prio is not const.
Why do we need such intermediate variable?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not necessary. but I like giving it an intuitive name. it probably should be const, but it's a copy either way.


// in case there's an error, we make sure m_file_priority is only
// updated for the successful files. By leaving failed files as
// priority 0, we allow re-trying them.
auto restore_prio = aux::scope_end([&] {
m_file_priority[i] = old_prio;
prio = m_file_priority;
});

if (old_prio == dont_download && new_prio != dont_download)
{
// move stuff out of the part file
std::shared_ptr<aux::file_mapping> f = open_file(sett, i, aux::open_mode::write, ec);
if (ec)
{
prio = m_file_priority;
return;
}
if (ec) return;

if (m_part_file && use_partfile(i))
{
Expand All @@ -196,7 +204,6 @@ error_code translate_error(std::error_code const& err, bool const write)
{
ec.file(i);
ec.operation = operation_t::partfile_write;
prio = m_file_priority;
return;
}
}
Expand Down Expand Up @@ -228,19 +235,14 @@ error_code translate_error(std::error_code const& err, bool const write)
{
ec.file(i);
ec.operation = operation_t::file_stat;
prio = m_file_priority;
return;
}
use_partfile(i, !file_exists);
/*
auto f = open_file(sett, i, aux::open_mode::read_only, ec);
if (ec.ec != boost::system::errc::no_such_file_or_directory)
{
if (ec)
{
prio = m_file_priority;
return;
}
if (ec) return;

need_partfile();

Expand All @@ -249,7 +251,6 @@ error_code translate_error(std::error_code const& err, bool const write)
{
ec.file(i);
ec.operation = operation_t::partfile_read;
prio = m_file_priority;
return;
}
// remove the file
Expand All @@ -259,14 +260,13 @@ error_code translate_error(std::error_code const& err, bool const write)
{
ec.file(i);
ec.operation = operation_t::file_remove;
prio = m_file_priority;
return;
}
}
*/
}
ec.ec.clear();
m_file_priority[i] = new_prio;
restore_prio.disarm();

if (m_file_priority[i] == dont_download && use_partfile(i))
{
Expand Down
Loading