ikatson / rqbit

A bittorrent client in Rust
Other
861 stars 82 forks source link

PermissionDenied when deleting .bitv #234

Open CarrotRub opened 2 months ago

CarrotRub commented 2 months ago
2024-09-11T14:59:55.516739Z  WARN librqbit::session_persistence::json: error removing error=Os { code: 5, kind: PermissionDenied, message: "Access is denied." } filename="C:\\Users\\CarrotRub\\AppData\\Local\\com.fitlauncher.carrotrub\\.persistence\\b13e432445bd2cadc5c7a97400376cba54f9fcb4.bitv"

When trying to download a torrent that's technically in persistence but has already been deleted, it will directly fill in the space with blank data causing issues and making the downloaded torrent unusable, it seems that it cannot delete the .bitv due to Access is Denied but the app is being launched as an admin.

I suspect the error to come from there but I may totally be wrong.

    async fn clear(&self, id: TorrentIdOrHash) -> anyhow::Result<()> {
        let h = self.to_hash(id).await?;
        let filename = self.bitv_filename(&h);
        tokio::fs::remove_file(&filename)
            .await
            .with_context(|| format!("error removing {filename:?}"))
    }

And btw I love your project so much it is really greatly designed and well done, good job man.

CarrotRub commented 2 months ago

Okay so the issue may be when I initialize the session, it directly place the folder of the downloaded torrent with blank data back even when it has been deleted already.

And when i initialize it, in my code I first get the details of the torrent before starting it so even though I try to delete the old torrents later on, it refuses.


             let persistence_config = Some(SessionPersistenceConfig::Json {
                folder: Some(persistence_path.into()),
            });

            let mut custom_session_options = SessionOptions::default();
            custom_session_options.disable_dht= false;
            custom_session_options.disable_dht_persistence= true;
            custom_session_options.fastresume= true;
            custom_session_options.listen_port_range= Some(6881..6889);
            custom_session_options.enable_upnp_port_forwarding= true;
            custom_session_options.defer_writes_up_to= None; 
            custom_session_options.concurrent_init_limit= Some(1);
            custom_session_options.persistence = persistence_config ;

            let session_global = match Session::new_with_opts(download_path.into(), custom_session_options).await {
                Ok(session) => session,
                Err(err) => {
                    error!("Error while creating a new session: {:#?}", err);
                    return Err(Box::new(TorrentError::AnyhowError(err.to_string())));
                }
ikatson commented 2 months ago

There might be multiple issues here, but let's clarify them.

When trying to download a torrent that's technically in persistence but has already been deleted

Can you clarify what does that mean and how to reproduce it? If the torrent is deleted, it should be removed from persistence also (JSON backend by default).

it will directly fill in the space with blank data causing issues and making the downloaded torrent unusable

the file space itself? This is very weird and shouldn't happen. The only place that writes to the file is when a piece is downloaded from the peer. Otherwise the files are created and length is set on them, so if the file is empty, it will look like it's filled with zeroes.

it seems that it cannot delete the .bitv due to Access is Denied but the app is being launched as an admin.

That may be a windows thing (which I didn't test on cause I don't have one). I can hypothesize that the bitvector (the mmaped .bitv file) needs to be closed first. But other than leaving unwanted files in the filesystem it shouldn't be a problem, at least in principle.

And btw I love your project so much it is really greatly designed and well done, good job man. Thank you, that's very good to hear!

it directly place the folder of the downloaded torrent with blank data back even when it has been deleted already.

that could only happen if the torrent wasn't actually deleted from persistence for some reason.

Can you trying doing all that with trace logs?

Also would be cool if you describe the exact sequence of steps to reproduce this

CarrotRub commented 2 months ago

1 - After downloading a torrent, the user might choose to start another download without removing the previous one from the session but still deleting the files of the previous torrent. So in my code I added a function to remove them from the session and they are removed automatically from the JSON file.

2024-09-11T21:48:26.033126Z ERROR torrent{id=0}:manage_peer{peer="51.159.29.239:51201"}: librqbit_core::spawn_utils: finished with error: chunk tracker empty, torrent was paused
2024-09-11T21:48:26.033395Z  WARN librqbit::session_persistence::json: error removing error=Os { code: 5, kind: PermissionDenied, message: "Access is denied." } filename="C:\\Users\\CarrotRub\\AppData\\Local\\com.fitlauncher.carrotrub\\.persistence\\b13e432445bd2cadc5c7a97400376cba54f9fcb4.bitv"
2024-09-11T21:48:26.034811Z ERROR torrent{id=0}:manage_peer{peer="105.186.226.64:42276"}: librqbit_core::spawn_utils: finished with error: chunk tracker empty, torrent was paused
2024-09-11T21:48:26.035128Z  INFO librqbit::session: deleted torrent id=0

2 - Yes the files are blank and if for example 400miB has been downloaded but the files have been deleted, it will start at 400miB of blank data that will corrupt everything.

3 - I will try to make a somewhat reproducible example :

        pub async fn new(
            download_path: String, 
            app_cache_patch: String
        ) -> Result<Self, Box<TorrentError>> {

            //  This line is ugly and really bad.
            let persistence_path = format!("{}.persistence", app_cache_patch).replace("/", "\\");

            let _dht_persistence_path = format!("{}.dht_persistence/", app_cache_patch);

            let persistence_config = Some(SessionPersistenceConfig::Json {
                folder: Some(persistence_path.into()),
            });

            // Not working, to be added later.
            // let _personal_dht_config = Some(PersistentDhtConfig {
                // config_filename: Some(persistence_path),
                // ..Default::default()
            // });

            // TODO: Add a way to either ask the user to choose between HDD or SSD (For a different config)

            let mut custom_session_options = SessionOptions::default();
            custom_session_options.disable_dht= false;
            custom_session_options.disable_dht_persistence= true;
            // custom_session_options.dht_config= personal_dht_config;
            custom_session_options.fastresume= true;
            custom_session_options.listen_port_range= Some(6881..6889);
            custom_session_options.enable_upnp_port_forwarding= true;
            custom_session_options.defer_writes_up_to= None; // Should defer up to 8~4mB for slow disk.
            custom_session_options.concurrent_init_limit= Some(1);
            custom_session_options.persistence = persistence_config ;

            let session_global = match Session::new_with_opts(download_path.into(), custom_session_options).await {
                Ok(session) => session,
                Err(err) => {
                    error!("Error while creating a new session: {:#?}", err);
                    return Err(Box::new(TorrentError::AnyhowError(err.to_string())));
                }
            };

            let (tx, _rx) = mpsc::unbounded_channel();
            let option_tx: Option<mpsc::UnboundedSender<String>> = Some(tx);
            let cus_api_session = Arc::new(Api::new(session_global.clone(), option_tx));

            Ok(TorrentManager {     
                api_session: cus_api_session
            })
        }

Here, when I initialize the session using this function, it directly place the blank data if something is in the session even if it has been deleted from session.json

And also if I delete the .bitv file manually, it fixes the issue.

ikatson commented 2 months ago

Sorry, I don't understand still, as the code isn't complete - you aren't downloading any torrent, and aren't deleting anything there.

Can you specify a sequence of steps e.g. is this one correct?

  1. add a torrent
  2. session.delete(torrent, with_files=true)
  3. add it again
  4. everything is broken

If your issue is that bitv file needs to be erased if you delete the underlying files, it was addressed recently in https://github.com/ikatson/rqbit/pull/228, make sure you have that code. What's the librqbit version / commit you're using?

CarrotRub commented 2 months ago

So I was using the 7.0.1 version and I tried to upgrade to the version you gave me and when I did so, the error is still there but I got new logs :

2024-09-12T12:26:51.131021Z  WARN torrent{id=0}:initialize_and_start: librqbit::file_ops: the piece=268 hash does not match
2024-09-12T12:26:51.141183Z  WARN torrent{id=0}:initialize_and_start: librqbit::file_ops: the piece=265 hash does not match
2024-09-12T12:26:51.152497Z  WARN torrent{id=0}:initialize_and_start: librqbit::file_ops: the piece=116 hash does not match
2024-09-12T12:26:51.163145Z  WARN torrent{id=0}:initialize_and_start: librqbit::file_ops: the piece=315 hash does not match
2024-09-12T12:26:51.173315Z  WARN torrent{id=0}:initialize_and_start: librqbit::file_ops: the piece=62 hash does not match
2024-09-12T12:26:51.174773Z  INFO torrent{id=0}:initialize_and_start: librqbit::torrent_state::initializing: Initial check results: have 1.2Gi, needed 760.0Mi, total selected 2.0Gi
2024-09-12T12:26:59.957341Z ERROR torrent{id=0}:manage_peer{peer="209.216.127.28:55231"}: librqbit_core::spawn_utils: finished with error: chunk tracker empty, torrent was paused
2024-09-12T12:26:59.957512Z ERROR torrent{id=0}:manage_peer{peer="95.181.238.54:39449"}: librqbit_core::spawn_utils: finished with error: chunk tracker empty, torrent was paused
2024-09-12T12:26:59.959486Z ERROR torrent{id=0}:manage_peer{peer="93.108.195.103:59150"}: librqbit_core::spawn_utils: finished with error: chunk tracker empty, torrent was paused
2024-09-12T12:26:59.961413Z  WARN librqbit::session_persistence::json: error removing error=Os { code: 5, kind: PermissionDenied, message: "Access is denied." } filename="C:\\Users\\CarrotRub\\AppData\\Local\\com.fitlauncher.carrotrub\\.persistence\\b13e432445bd2cadc5c7a97400376cba54f9fcb4.bitv"

Sorry, for not sending everything, I will try :

    impl TorrentManager {

        pub async fn new(
            download_path: String, 
            app_cache_patch: String
        ) -> Result<Self, Box<TorrentError>> {

            // Create the persistence path by appending ".persistence"
            let persistence_path = format!("{}.persistence", app_cache_patch).replace("/", "\\");

            let _dht_persistence_path = format!("{}.dht_persistence/", app_cache_patch);

            let persistence_config = Some(SessionPersistenceConfig::Json {
                folder: Some(persistence_path.into()),
            });

            // Not working, to be added later.
            // let _personal_dht_config = Some(PersistentDhtConfig {
                // config_filename: Some(persistence_path),
                // ..Default::default()
            // });

            // TODO: Add a way to either ask the user to choose between HDD or SSD (For a different config)

            let mut custom_session_options = SessionOptions::default();
            custom_session_options.disable_dht= false;
            custom_session_options.disable_dht_persistence= true;
            // custom_session_options.dht_config= personal_dht_config;
            custom_session_options.fastresume= true;
            custom_session_options.listen_port_range= Some(6881..6889);
            custom_session_options.enable_upnp_port_forwarding= true;
            custom_session_options.defer_writes_up_to= None; // Should defer up to 4mB for slow disk.
            custom_session_options.concurrent_init_limit= Some(1);
            custom_session_options.persistence = persistence_config ;

            let session_global = match Session::new_with_opts(download_path.into(), custom_session_options).await {
                Ok(session) => session,
                Err(err) => {
                    error!("Error while creating a new session: {:#?}", err);
                    return Err(Box::new(TorrentError::AnyhowError(err.to_string())));
                }
            };

            let (tx, _rx) = mpsc::unbounded_channel();
            let option_tx: Option<mpsc::UnboundedSender<String>> = Some(tx);
            let cus_api_session = Arc::new(Api::new(session_global.clone(), option_tx));

            Ok(TorrentManager {     
                api_session: cus_api_session
            })
        }

        pub async fn download_torrent_with_args(
            &self,
            magnet_link: String, 
            file_list: Vec<usize>
        ) -> Result<(), Box<TorrentError>> {

            // First, delete all previous torrents from the session persistence to avoid any re-download, may be removed later when multiple downloads will be added but the concurrent_init_limit should be raised too.

            // Get the list response.
            let torrent_list_response = Api::api_torrent_list(&self.api_session);

            // Get the hash of the torrent that will be downloaded.
            let actual_torrent_magnet = match Magnet::parse(&magnet_link) {
                Ok(magnet) => magnet,
                Err(e) => {
                    error!("Error Parsing Magnet : {:#?}", e);
                    return Err(Box::new(TorrentError::AnyhowError("Error forgetting the torrent from the session persistence.".to_string())).into());
                }
            };

            let actual_torrent_id20 = Magnet::as_id20(&actual_torrent_magnet);

            let actual_torrent_hash = TorrentIdOrHash::Hash(actual_torrent_id20.unwrap()).to_string();

            if !torrent_list_response.torrents.is_empty() {
                // Get the vector and transform it into an Iterator
                let torrent_list_iter = torrent_list_response.torrents.iter();

                for torrent in torrent_list_iter {

                    if torrent.info_hash != actual_torrent_hash {
                        match self.stop_torrent(torrent.info_hash.clone()).await {
                            Ok(()) => {
                                info!("Forgot the torrent ID : {:#?} | Hash : {:#?}", &torrent.id, &torrent.info_hash);
                            },
                            Err(e) => {
                                error!("Error forgetting the torrent from the session persistence : {:#?}", e);
                                return Err(Box::new(TorrentError::AnyhowError("Error forgetting the torrent from the session persistence.".to_string())).into());
                            }
                        }
                    }
                }
            }

            let _torrent_response = match Api::api_add_torrent(
                &self.api_session,
                AddTorrent::from_url(&magnet_link),
                Some(AddTorrentOptions {
                    only_files: Some(file_list),
                    overwrite: true,
                    disable_trackers: false,
                    force_tracker_interval: Some(Duration::from_secs(30)), // Check tracker every minute
                    ..Default::default()
                })
            ).await {
                Ok(response) => {
                    info!("Torrent Was Successfully Added And The Installation Successfully Started");
                    println!("Torrent Was Successfully Added And The Installation Successfully Started");

                    Some(response)
                },
                Err(e) => {
                    error!("Error While Adding The Torrent To Download : {}", e);
                    None
                }
            };

            Ok(())
        }

    }

For the sequence of steps :

1 - Add a torrent named X. 2 - Close the app. 3 - Delete the files of X. 4 - Add a new torrent named Y. 5 - The files of X reappears and are blank and corrupt. 6 - The files of Y start downloading.

There is also this sequence that is broken :

1 - Add a torrent named X. 2 - Close the app. 3 - Delete the files of X. 4 - Add again a torrent named X. 5 - The files of X reappears and are blank and corrupt. 6 - The files of X start downloading and continuing from the corrupt data, making the final result unusable.

And sorry for taking a long time to reply, I was trying to make it work.

ikatson commented 2 months ago

5 - The files of X reappears and are blank and corrupt.

What is the expectation here? As you deleted the files beforehand, and rqbit didn't know about it, as soon as you restart rqbit, it recreates the files and will download them again (unless you paused the torrent in which case it will only recreate the files, but not download them until you unpause).

CarrotRub commented 2 months ago

I thought it wouldn't recreate the files with blank data but will just start from 0 instead. And also even if the torrent isn't in the session.json anymore, it still places the blank data probably due to the bitv not deleting because as soon as I delete the bitv it gets back from 0 instead of recreating blank data.

ikatson commented 2 months ago

but will just start from 0 instead

Can you clarify this? Do you mean your expectation is that it will create files with 0 length? If so, it does that at first, but then calls set_length() on it, so that it can pwrite() to arbitrary places later when pieces come in. Depending on the OS, it may or may not consume actual disk space.

I don't think it has anything to do with the .bitv file though

CarrotRub commented 2 months ago

As you can see there :

2024-09-12T12:26:51.174773Z  INFO torrent{id=0}:initialize_and_start: librqbit::torrent_state::initializing: Initial check results: have 1.2Gi, needed 760.0Mi, total selected 2.0Gi

It started downloading at the point where it left at previously but since the file was already deleted it filled it with blank space. And yes I may be wrong about the .bitv file but I had doubts about it because he is the one not being deleted like seen here :

2024-09-12T12:39:58.901284Z  WARN librqbit::session_persistence::json: error removing error=Os { code: 5, kind: PermissionDenied, message: "Access is denied." } filename="C:\\Users\\CarrotRub\\AppData\\Local\\com.fitlauncher.carrotrub\\.persistence\\6cab6f4bebe26b28fa05f50fc3b4878f279208bc.bitv"

And when it is deleted manually, it fixes the issue.

ikatson commented 2 months ago

I think I understand what you mean now, please confirm if that's correct.

The torrent has many files. But you deleted only some of them after they were downloaded. rqbit (with fastresume enabled) doesn't detect this when re-adding the torrent, and thinks you have more pieces than you actually have (because it loads the old .bitv file). Is that correct?

ikatson commented 2 months ago

Can you check this branch https://github.com/ikatson/rqbit/pull/235? If I got you correctly, this should detect missing files even when fastresume is enabled, as it checks at least one piece from each file.

It should print "data corrupted, ignoring fastresume data" in your case.

CarrotRub commented 2 months ago

Yes it is, but it doesn't have to be only some files, it can be all the files. But you are 100% correct, I tried disabling fastresume and it approximately worked, meaning the old deleted files appear but they are still at 0kB so they are not corrupt and cause no issue and the download start normally but I still love the fastresume feature so if there is a fix to it I would like to hear it.

Thank you again for helping me !

CarrotRub commented 2 months ago

I will check it and tell you if it works.

CarrotRub commented 2 months ago

It works great with the fast resume now, even though the folder of the old torrent and its files are still created, they are all at 0 kB and it is even better since the fast resume works on them when being downloaded again.

Thank you for creating this project, it is one of the best dependency I worked on, even the source code is really readable and understandable, keep up the good work man and good luck !!

ikatson commented 2 months ago

I think I also fixed removing the .bitv file on Windows. Pushed to the same PR: https://github.com/ikatson/rqbit/pull/235

Can you please check it too.

it is even better since the fast resume works on them when being downloaded again.

I don't think it works with fastresume (for the broken torrent at least), because what the new code does is validates if fastresume data is legit. And as it's not legit, it returns to validating the torrent the old way. But at least other torrents will work with fastresume fine.

CarrotRub commented 2 months ago

I will check it right now.

CarrotRub commented 2 months ago

Everything works PERFECTLY, you are an absolute GOAT. There are no issues anymore, fastresume is on, there no leftover blank data, nothing. It is PERFECT, you are the best man.

Thank you so much and this time I checked and I didn't forget anything haha, it all works and there is no errors concerning the .bitv nor any warning about a hash that does not match !

Good job and thank you for maintaining this project, much love <3

ikatson commented 2 months ago

Merged into main and released in 7.1.0-beta.1

CarrotRub commented 1 month ago

I don't know if I should open a new issue for this but after using specifically the tag = "v7.1.0-beta.1" I came upon an issue that wasn't here in the version 7.0.1, it concern the bitv renaming this time

2024-10-07T18:57:14.296847Z ERROR torrent{id=0}:initialize_and_start: librqbit_core::spawn_utils: finished with error: error storing initial check bitfield

Caused by:
    0: error renaming "C:\\Users\\CarrotRub\\AppData\\Local\\com.fitlauncher.carrotrub\\.persistence\\64819539d081c7cdc2d48c7382c1cbd45b82d664.bitv.tmp" to "C:\\Users\\CarrotRub\\AppData\\Local\\com.fitlauncher.carrotrub\\.persistence\\64819539d081c7cdc2d48c7382c1cbd45b82d664.bitv"
    1: Access is denied. (os error 5)

Steps to reproduce :

  1. Initialize the session.
  2. Add a torrent to a session and wait until it starts downloading a little bit.
  3. Pause the torrent and close the app.
  4. Open the app and initialize the session.
  5. Try to resume the torrent using Api::api_torrent_action_start
  6. The error arise.

This does not happen in the v7.0.1

ikatson commented 3 days ago

@CarrotRub I can't reproduce this on main branch on Windows.

I'm running the server with cargo run -- server start --fastresume /tmp/scratch, and controlling the torrent through Web UI. Everything works fine.

Can you check if main branch is broken for you?

CarrotRub commented 3 days ago

The main branch doesn't have this issue but the v7.1.0-beta.1 has this issue (The one where you fixed an issue related with fastresume). So it is probably related to the fix made.

ikatson commented 3 days ago

@CarrotRub if it works fine on main, there's nothing in 7.1.0-beta.1 that isn't merged into main afaik.

So can we just close it? Or you need a release

CarrotRub commented 16 hours ago

I have tried again and now, the renaming issue happens on branch main too, I apologize for the confusion. I tried the branch main and the tag v7.1.0-beta.1, I will try to see if the issue happens due to my code and if not, I will try to come up with a solution and push a PR