Workaround for Large File Corruption During Save #4276

ngraham20 · 2022-10-14T20:23:28Z

PR as a workaround for #3967

ngraham20 · 2022-10-14T20:25:57Z

I could also imagine

tokio::fs::rename(&tmp_path, path).await?;

being used instead of a copy and remove

https://docs.rs/tokio/latest/tokio/fs/fn.rename.html

kirawi · 2022-10-15T00:40:46Z

Using rename is probably better since copy will lead to the same issue. I think it would also be better to create the temp file in the cache directory.

ngraham20 · 2022-10-15T01:48:33Z

I went the route of making the cached file match the filename instead of having a file called "cachefile" or something just in case multiple saves and crashes occur at the same time

helix-view/src/document.rs

*facepalm*

ngraham20 · 2022-10-15T02:22:35Z

I don't get why the builds are failing. The file saves to exactly where it's supposed to. Could it potentially be an async thing, where it's checking too quickly and the file hasn't been copied to the new location yet?

kirawi · 2022-10-15T02:24:34Z

I'm not sure. The async operation should be completed successfully before it's checked.
@dead10ck

kirawi · 2022-10-15T02:37:12Z

Since it's writing to the cache folder, maybe the CI doesn't allow that?

ngraham20 · 2022-10-15T02:39:07Z

Since it's writing to the cache folder, maybe the CI doesn't allow that?

Maybe? But I would assume it isn't looking there, it should be looking for the file that got created in the working directory? In which case, it saves to the temp file, then moves it to the correct location, then the assert should look there, yeah?

ngraham20 · 2022-10-15T02:39:46Z

oh you mean that it could be failing to write to the cache folder, therefore it never gets moved into the correct spot afterwards... That makes sense to me

kirawi · 2022-10-15T02:39:48Z

If it wasn't able to write to the cache directory, then the write operation would fail. I'm assuming that's why the file is empty.

ngraham20 · 2022-10-15T02:41:19Z

If it wasn't able to write to the cache directory, then the write operation would fail. I'm assuming that's why the file is empty.

wait, no, because I ran the integration tests locally just now and they still fail, so it can't be that

kirawi · 2022-10-15T02:42:45Z

Could you test with reverting the rename() change?

ngraham20 · 2022-10-15T02:46:11Z

Could you test with reverting the rename() change?

yeah that made it pass

super weird

ngraham20 · 2022-10-15T02:48:04Z

I'm going to upload this version for now. Maybe dead10ck has some idea of what's going on, I sure don't 😅

…20/helix into workaround-large-file-corruption

dead10ck · 2022-10-15T02:55:01Z

This will not work if the new name is on a different mount point.

Interesting. That's probably what it is. The test is making a temporary file in /tmp, which is often on a different mount point than /home. That's an odd and unfortunate limitation. Copying doubles the amount of IO for each file save.

Edit: nope, spoke too soon. Could you change the log level to debug here?

ngraham20 · 2022-10-15T03:02:13Z

hmmmm, the integration test is still failing. Works when I run them locally though, even with the copy/delete

ngraham20 · 2022-10-18T17:48:27Z

Yeah I came to the same conclusion after more testing. Bummer haha

ngraham20 · 2022-10-18T17:49:33Z

Makes it even more confusing though why it would fail to read the contents, as clearly its getting renamed properly and absolutely has the correct string in it.

ngraham20 · 2022-10-18T17:51:19Z

something fundamentally has to be different between copy and rename.

ngraham20 · 2022-10-18T18:01:06Z

2022-10-18T11:58:50.285 helix_view::document [INFO] actual file ino: 979359                                                    
2022-10-18T11:58:50.286 helix_view::document [INFO] before rename                                                              
2022-10-18T11:58:50.286 helix_view::document [INFO] tmpfile exist: true                                                        
2022-10-18T11:58:50.286 helix_view::document [INFO] actual exist: true                                                         
2022-10-18T11:58:50.286 helix_view::document [INFO] tmpfile ino: 980418                                                        
2022-10-18T11:58:50.286 helix_view::document [INFO] actual ino: 979359                                                         
2022-10-18T11:58:50.286 helix_view::document [INFO] after rename                                                               
2022-10-18T11:58:50.286 helix_view::document [INFO] actual ino: 980418                                                         
2022-10-18T11:58:50.286 helix_view::document [INFO] tmpfile exist: false                                                       
2022-10-18T11:58:50.286 helix_view::document [INFO] actual exist: true                                                         
2022-10-18T11:58:50.293 integration::test::write [INFO] TEST:: ino after keysequence: 980418                                   
2022-10-18T11:58:50.295 integration::test::write [INFO] TEST:: ino of newfile: 979359

weird. If I clone the testfile after the rename, the cloned file has the original ino, not the new one...

kirawi · 2022-10-18T21:35:14Z

I wrote this unit test:

#[tokio::test]
    async fn save() {
        use std::io::Read;
        let mut file = tempfile::NamedTempFile::new().unwrap();
        let mut doc = Document::open(file.path(), None, None).unwrap();
        let view = ViewId::default();
        doc.set_selection(view, Selection::single(0, 0));
        let transaction = Transaction::change(doc.text(), [(0, 0, Some("ithe gostak distims the doshes".into()))].into_iter());
        doc.apply(&transaction, view);
        doc.save(false).await.unwrap();

        let mut file_content = String::new();
        file.as_file_mut().read_to_string(&mut file_content).unwrap();
        assert_eq!(file_content, "ithe gostak distims the doshes");
    }

And it fails with thread 'document::test::save' panicked at 'called Result::unwrap() on an Err value: Access is denied. (os error 5) originating from tokio::fs::rename(tmp_path, path).await?;

dead10ck · 2022-10-19T01:14:35Z

That is very confusing. I don't understand how it could have access to write a new file, but not to rename/overwrite.

ngraham20 · 2022-10-20T14:42:28Z

Maybe one of the earlier processes is keeping the file open? Permission denied could occur if another of the async functions still has it open for r/w right?

kirawi · 2022-10-20T15:03:57Z

I found this issue: Stebalien/tempfile#131 and it might be Windows-specific. Could you see what the panic is on a unix machine?

This passed:

#[tokio::test]
async fn save() {
    use std::io::Read;
    let file = tempfile::NamedTempFile::new().unwrap();
    // Need to close the temp file.
    let path = file.into_temp_path();
    let mut doc = Document::open(&path, None, None).unwrap();
    let view = ViewId::default();
    doc.set_selection(view, Selection::single(0, 0));
    let transaction = Transaction::insert(
        doc.text(),
        doc.selection(view),
        "ithe gostak distims the doshes".into(),
    );
    doc.apply(&transaction, view);
    doc.save(false).await.unwrap();

    let file_content = std::fs::read_to_string(path).unwrap();
    assert_eq!(file_content, "ithe gostak distims the doshes");
}

ngraham20 · 2022-10-20T15:26:54Z

I get this on Linux

---- test::write::save stdout ----
thread 'test::write::save' panicked at 'called `Result::unwrap()` on an `Err` value: Bad file descriptor (os error 9)', helix-term/tests/test/write.rs:29:27
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

#[tokio::test]
async fn save() {
    use helix_view::document::Document;
    use helix_core::Transaction;
    use helix_view::ViewId;
    use std::io::Read;
    let file = tempfile::NamedTempFile::new().unwrap();
    // Need to close the temp file.
    let path = file.into_temp_path();
    let mut doc = Document::open(&path, None, None).unwrap();
    let view = ViewId::default();
    doc.set_selection(view, Selection::single(0, 0));
    let transaction = Transaction::insert(
        doc.text(),
        doc.selection(view),
        "ithe gostak distims the doshes".into(),
    );
    doc.apply(&transaction, view);
    doc.save(false).await.unwrap();

    let file_content = std::fs::read_to_string(path).unwrap();
    assert_eq!(file_content, "ithe gostak distims the doshes");
}

kirawi · 2022-10-20T15:31:01Z

How about this?

#[tokio::test]
async fn save() {
    use std::io::Read;
    let file = tempfile::NamedTempFile::new().unwrap();
    let (_, path) = file.keep().unwrap();
    let mut doc = Document::open(&path, None, None).unwrap();
    let view = ViewId::default();
    doc.set_selection(view, Selection::single(0, 0));
    let transaction = Transaction::insert(
        doc.text(),
        doc.selection(view),
        "ithe gostak distims the doshes".into(),
    );
    doc.apply(&transaction, view);
    doc.save(false).await.unwrap();

    let file_content = std::fs::read_to_string(path).unwrap();
    assert_eq!(file_content, "ithe gostak distims the doshes");
}

ngraham20 · 2022-10-20T15:32:33Z

How about this?

same error. Specifically, it's failing on the save()

kirawi · 2022-10-20T15:48:22Z

Could you see where it originates from in the backtrace?

ngraham20 · 2022-10-20T15:58:55Z

Here's the basic stacktrace

---- test::write::save stdout ----
thread 'test::write::save' panicked at 'called `Result::unwrap()` on an `Err` value: Bad file descriptor (os error 9)', helix-term/tests/test/write.rs:28:27
stack backtrace:
   0: rust_begin_unwind
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:143:14
   2: core::result::unwrap_failed
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/result.rs:1785:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/result.rs:1078:23
   4: integration::test::write::save::{{closure}}
             at ./tests/test/write.rs:28:5
   5: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/future/mod.rs:91:19
   6: <core::pin::Pin<P> as core::future::future::Future>::poll
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/future/future.rs:124:9
   7: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:525:48
   8: tokio::coop::with_budget::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/coop.rs:102:9
   9: std::thread::local::LocalKey<T>::try_with
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/thread/local.rs:442:16
  10: std::thread::local::LocalKey<T>::with
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/thread/local.rs:418:9
  11: tokio::coop::with_budget
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/coop.rs:95:5
  12: tokio::coop::budget
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/coop.rs:72:5
  13: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:525:25
  14: tokio::runtime::scheduler::current_thread::Context::enter
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:349:19
  15: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:524:36
  16: tokio::runtime::scheduler::current_thread::CoreGuard::enter::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:595:57
  17: tokio::macros::scoped_tls::ScopedKey<T>::set
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/macros/scoped_tls.rs:61:9
  18: tokio::runtime::scheduler::current_thread::CoreGuard::enter
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:595:27
  19: tokio::runtime::scheduler::current_thread::CoreGuard::block_on
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:515:19
  20: tokio::runtime::scheduler::current_thread::CurrentThread::block_on
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:161:24
  21: tokio::runtime::Runtime::block_on
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/mod.rs:490:46
  22: integration::test::write::save
             at ./tests/test/write.rs:31:5
  23: integration::test::write::save::{{closure}}
             at ./tests/test/write.rs:12:7
  24: core::ops::function::FnOnce::call_once
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/ops/function.rs:227:5
  25: core::ops::function::FnOnce::call_once
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

kirawi · 2022-10-20T16:00:45Z

Could you try putting the test in document.rs? I don't think it's capturing the function calls there.

ngraham20 · 2022-10-20T16:10:34Z

Could you try putting the test in document.rs? I don't think it's capturing the function calls there.

sorry, I'm super unfamiliar with the testing framework 😅. I tried cargo test save and cargo test helix-view::document::save and neither of them ran the test. What command should I be using if the test is in document.rs?

ngraham20 · 2022-10-20T16:11:40Z

wait hangon

kirawi · 2022-10-20T16:12:40Z

cargo test --workspace save

ngraham20 · 2022-10-20T16:13:43Z

cargo test --workspace save

and I put the test in the test mod in document.rs?

ngraham20 · 2022-10-20T16:15:51Z

#[cfg(test)]
mod test {
    use super::*;

    #[tokio::test]
    async fn save() {
        use std::io::Read;
        let file = tempfile::NamedTempFile::new().unwrap();
        let (_, path) = file.keep().unwrap();
        let mut doc = Document::open(&path, None, None).unwrap();
        let view = ViewId::default();
        doc.set_selection(view, Selection::single(0, 0));
        let transaction = Transaction::insert(
            doc.text(),
            doc.selection(view),
            "ithe gostak distims the doshes".into(),
        );
        doc.apply(&transaction, view);
        doc.save(false).await.unwrap();

        let file_content = std::fs::read_to_string(path).unwrap();
        assert_eq!(file_content, "ithe gostak distims the doshes");
    }

error[E0433]: failed to resolve: use of undeclared crate or module `tempfile`
    --> helix-view/src/document.rs:1208:20
     |
1208 |         let file = tempfile::NamedTempFile::new().unwrap();
     |                    ^^^^^^^^ use of undeclared crate or module `tempfile`

warning: unused import: `std::io::Read`
    --> helix-view/src/document.rs:1207:13
     |
1207 |         use std::io::Read;
     |             ^^^^^^^^^^^^^
     |
     = note: `#[warn(unused_imports)]` on by default

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
    --> helix-view/src/document.rs:1209:17
     |
1209 |         let (_, path) = file.keep().unwrap();
     |                 ^^^^ doesn't have a size known at compile-time
     |
     = help: within `std::path::Path`, the trait `Sized` is not implemented for `[u8]`
     = note: required because it appears within the type `std::path::Path`
     = note: all local variables must have a statically known size
     = help: unsized locals are gated as an unstable feature

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
    --> helix-view/src/document.rs:1221:52
     |
1221 |         let file_content = std::fs::read_to_string(path).unwrap();
     |                            ----------------------- ^^^^ doesn't have a size known at compile-time
     |                            |
     |                            required by a bound introduced by this call
     |
     = help: within `std::path::Path`, the trait `Sized` is not implemented for `[u8]`
     = note: required because it appears within the type `std::path::Path`
note: required by a bound in `std::fs::read_to_string`
    --> /home/ngraham/.rustup/toolchains/1.61.0-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/fs.rs:267:23
     |
267  | pub fn read_to_string<P: AsRef<Path>>(path: P) -> io::Result<String> {
     |                       ^ required by this bound in `std::fs::read_to_string`

Some errors have detailed explanations: E0277, E0433.
For more information about an error, try `rustc --explain E0277`.
warning: `helix-view` (lib test) generated 1 warning
error: could not compile `helix-view` due to 3 previous errors; 1 warning emitted
warning: build failed, waiting for other jobs to finish...

kirawi · 2022-10-20T16:17:19Z

You also have to add tempfile to the helix-view dependencies.

ngraham20 · 2022-10-20T16:20:52Z

Thanks for being patient with me 😅

The test fails for the same reason in here

running 1 test
test document::test::save ... FAILED

failures:

---- document::test::save stdout ----
thread 'document::test::save' panicked at 'called `Result::unwrap()` on an `Err` value: Bad file descriptor (os error 9)', helix-view/src/document.rs:1219:31
stack backtrace:
   0: rust_begin_unwind
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:143:14
   2: core::result::unwrap_failed
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/result.rs:1785:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/result.rs:1078:23
   4: helix_view::document::test::save::{{closure}}
             at ./src/document.rs:1219:9
   5: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/future/mod.rs:91:19
   6: <core::pin::Pin<P> as core::future::future::Future>::poll
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/future/future.rs:124:9
   7: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:525:48
   8: tokio::coop::with_budget::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/coop.rs:102:9
   9: std::thread::local::LocalKey<T>::try_with
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/thread/local.rs:442:16
  10: std::thread::local::LocalKey<T>::with
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/thread/local.rs:418:9
  11: tokio::coop::with_budget
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/coop.rs:95:5
  12: tokio::coop::budget
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/coop.rs:72:5
  13: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:525:25
  14: tokio::runtime::scheduler::current_thread::Context::enter
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:349:19
  15: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:524:36
  16: tokio::runtime::scheduler::current_thread::CoreGuard::enter::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:595:57
  17: tokio::macros::scoped_tls::ScopedKey<T>::set
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/macros/scoped_tls.rs:61:9
  18: tokio::runtime::scheduler::current_thread::CoreGuard::enter
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:595:27
  19: tokio::runtime::scheduler::current_thread::CoreGuard::block_on
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:515:19
  20: tokio::runtime::scheduler::current_thread::CurrentThread::block_on
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:161:24
  21: tokio::runtime::Runtime::block_on
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/mod.rs:490:46
  22: helix_view::document::test::save
             at ./src/document.rs:1222:9
  23: helix_view::document::test::save::{{closure}}
             at ./src/document.rs:1206:11
  24: core::ops::function::FnOnce::call_once
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/ops/function.rs:227:5
  25: core::ops::function::FnOnce::call_once
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


failures:
    document::test::save

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 39 filtered out; finished in 0.03s

alois31 · 2022-11-26T13:13:41Z

I wrote this unit test:

#[tokio::test]
    async fn save() {
        use std::io::Read;
        let mut file = tempfile::NamedTempFile::new().unwrap();
        let mut doc = Document::open(file.path(), None, None).unwrap();
        let view = ViewId::default();
        doc.set_selection(view, Selection::single(0, 0));
        let transaction = Transaction::change(doc.text(), [(0, 0, Some("ithe gostak distims the doshes".into()))].into_iter());
        doc.apply(&transaction, view);
        doc.save(false).await.unwrap();

        let mut file_content = String::new();
        file.as_file_mut().read_to_string(&mut file_content).unwrap();
        assert_eq!(file_content, "ithe gostak distims the doshes");
    }

And it fails with thread 'document::test::save' panicked at 'called Result::unwrap() on an Err value: Access is denied. (os error 5) originating from tokio::fs::rename(tmp_path, path).await?;

This is on Windows, right? The unit test is keeping the original file open, and on Windows, IIRC deleting or overwriting an open file is not allowed.

alois31 · 2022-11-26T13:14:18Z

Here's the basic stacktrace

---- test::write::save stdout ----
thread 'test::write::save' panicked at 'called `Result::unwrap()` on an `Err` value: Bad file descriptor (os error 9)', helix-term/tests/test/write.rs:28:27
stack backtrace:
   0: rust_begin_unwind
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:143:14
   2: core::result::unwrap_failed
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/result.rs:1785:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/result.rs:1078:23
   4: integration::test::write::save::{{closure}}
             at ./tests/test/write.rs:28:5
   5: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/future/mod.rs:91:19
   6: <core::pin::Pin<P> as core::future::future::Future>::poll
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/future/future.rs:124:9
   7: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:525:48
   8: tokio::coop::with_budget::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/coop.rs:102:9
   9: std::thread::local::LocalKey<T>::try_with
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/thread/local.rs:442:16
  10: std::thread::local::LocalKey<T>::with
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/thread/local.rs:418:9
  11: tokio::coop::with_budget
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/coop.rs:95:5
  12: tokio::coop::budget
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/coop.rs:72:5
  13: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:525:25
  14: tokio::runtime::scheduler::current_thread::Context::enter
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:349:19
  15: tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:524:36
  16: tokio::runtime::scheduler::current_thread::CoreGuard::enter::{{closure}}
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:595:57
  17: tokio::macros::scoped_tls::ScopedKey<T>::set
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/macros/scoped_tls.rs:61:9
  18: tokio::runtime::scheduler::current_thread::CoreGuard::enter
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:595:27
  19: tokio::runtime::scheduler::current_thread::CoreGuard::block_on
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:515:19
  20: tokio::runtime::scheduler::current_thread::CurrentThread::block_on
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/scheduler/current_thread.rs:161:24
  21: tokio::runtime::Runtime::block_on
             at /home/ngraham/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/tokio-1.21.2/src/runtime/mod.rs:490:46
  22: integration::test::write::save
             at ./tests/test/write.rs:31:5
  23: integration::test::write::save::{{closure}}
             at ./tests/test/write.rs:12:7
  24: core::ops::function::FnOnce::call_once
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/ops/function.rs:227:5
  25: core::ops::function::FnOnce::call_once
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

The debugging code attempts to read from a write-only file descriptor.

kirawi · 2024-01-04T05:25:43Z

I think I figured out the issue since I dealt with the same thing for persistent undo... turns out you need to seek to the beginning of the file after any read or write operation otherwise you will read a blank string.

kirawi · 2024-01-04T19:39:51Z

Superseded by #9236

Workaround for Large File Corruption During Save

e5ec444

kirawi added A-helix-term Area: Helix term improvements S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. C-bug Category: This is a bug labels Oct 15, 2022

Cache file in ~/.cache/helix

4bf41ba

kirawi reviewed Oct 15, 2022

View reviewed changes

helix-view/src/document.rs Outdated Show resolved Hide resolved

kirawi reviewed Oct 15, 2022

View reviewed changes

helix-view/src/document.rs Outdated Show resolved Hide resolved

ngraham20 added 3 commits October 14, 2022 20:09

Shorter cache folder retrieval

ee222fc

Removed to_str() for non-utf8 unix filenames

946e73b

Delete temp.txt

96d0018

*facepalm*

kirawi added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 15, 2022

ngraham20 added 2 commits October 14, 2022 20:48

Changed rename to copy and delete

7bb56dc

Merge branch 'workaround-large-file-corruption' of github.com:ngraham…

0a44c9f

…20/helix into workaround-large-file-corruption

Changed HELIX_LOG_LEVEL to debug

60ffe22

dead10ck mentioned this pull request Oct 27, 2022

Crashes when write-quitting a large file. #3975

Closed

kirawi mentioned this pull request Sep 15, 2023

Panic handler, that saves unsaved work to temporary files #290

Open

kirawi mentioned this pull request Jan 4, 2024

Use a temporary file for writes #9236

Merged

kirawi closed this Jan 4, 2024

ngraham20 deleted the workaround-large-file-corruption branch April 1, 2024 00:24

Workaround for Large File Corruption During Save #4276

Workaround for Large File Corruption During Save #4276

Conversation

ngraham20 commented Oct 14, 2022

ngraham20 commented Oct 14, 2022

kirawi commented Oct 15, 2022

ngraham20 commented Oct 15, 2022

ngraham20 commented Oct 15, 2022

kirawi commented Oct 15, 2022 • edited Loading

kirawi commented Oct 15, 2022 • edited Loading

ngraham20 commented Oct 15, 2022

ngraham20 commented Oct 15, 2022

kirawi commented Oct 15, 2022

ngraham20 commented Oct 15, 2022

kirawi commented Oct 15, 2022

ngraham20 commented Oct 15, 2022 • edited Loading

ngraham20 commented Oct 15, 2022

dead10ck commented Oct 15, 2022 • edited Loading

ngraham20 commented Oct 15, 2022

ngraham20 commented Oct 18, 2022

ngraham20 commented Oct 18, 2022

ngraham20 commented Oct 18, 2022

ngraham20 commented Oct 18, 2022

kirawi commented Oct 18, 2022

dead10ck commented Oct 19, 2022

ngraham20 commented Oct 20, 2022

kirawi commented Oct 20, 2022 • edited Loading

ngraham20 commented Oct 20, 2022 • edited Loading

kirawi commented Oct 20, 2022

ngraham20 commented Oct 20, 2022 • edited Loading

kirawi commented Oct 20, 2022

ngraham20 commented Oct 20, 2022

kirawi commented Oct 20, 2022

ngraham20 commented Oct 20, 2022

ngraham20 commented Oct 20, 2022

kirawi commented Oct 20, 2022

ngraham20 commented Oct 20, 2022

ngraham20 commented Oct 20, 2022

kirawi commented Oct 20, 2022

ngraham20 commented Oct 20, 2022

alois31 commented Nov 26, 2022

alois31 commented Nov 26, 2022

kirawi commented Jan 4, 2024

kirawi commented Jan 4, 2024

kirawi commented Oct 15, 2022 •

edited

Loading

kirawi commented Oct 15, 2022 •

edited

Loading

ngraham20 commented Oct 15, 2022 •

edited

Loading

dead10ck commented Oct 15, 2022 •

edited

Loading

kirawi commented Oct 20, 2022 •

edited

Loading

ngraham20 commented Oct 20, 2022 •

edited

Loading

ngraham20 commented Oct 20, 2022 •

edited

Loading