Skip to content

Commit

Permalink
Attempt to fix the flake Text file busy (os error 26) error in CI
Browse files Browse the repository at this point in the history
  • Loading branch information
@allada
allada committed Sep 15, 2023
1 parent 59bbfd8 commit b730a90
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 71 deletions.
7 changes: 0 additions & 7 deletions cas/store/filesystem_store.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,13 +519,6 @@ impl<Fe: FileEntry> FilesystemStore<Fe> {
file_size += data_len as u64;
}

resumeable_temp_file
.as_writer()
.await
.err_tip(|| "in filesystem_store::update_file")?
.flush()
.await
.err_tip(|| "Could not flush file in filesystem store")?;
resumeable_temp_file
.as_writer()
.await
Expand Down
1 change: 0 additions & 1 deletion cas/store/tests/filesystem_store_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ async fn write_file(file_name: &OsStr, data: &[u8]) -> Result<(), Error> {
.await
.err_tip(|| format!("Failed to create file: {file_name:?}"))?;
file.as_writer().await?.write_all(data).await?;
file.as_writer().await?.flush().await?;
file.as_writer()
.await?
.as_mut()
Expand Down
2 changes: 1 addition & 1 deletion cas/worker/running_actions_manager.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ impl RunningActionImpl {

let mut child_process = command_builder
.spawn()
.err_tip(|| format!("Could not execute command {:?}", command_proto.arguments))?;
.err_tip(|| format!("Could not execute command {:?}", args))?;
let mut stdout_reader = child_process
.stdout
.take()
Expand Down
28 changes: 16 additions & 12 deletions cas/worker/tests/local_worker_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use std::env;
use std::ffi::OsString;
#[cfg(target_family = "unix")]
use std::fs::Permissions;
use std::io::Write;
#[cfg(target_family = "unix")]
use std::os::unix::fs::PermissionsExt;
use std::path::PathBuf;
Expand Down Expand Up @@ -360,7 +361,8 @@ mod local_worker_tests {
fs::create_dir_all(format!("{}/{}", work_directory, "another_dir")).await?;
let mut file = fs::create_file(OsString::from(format!("{}/{}", work_directory, "foo.txt"))).await?;
file.as_writer().await?.write_all(b"Hello, world!").await?;
file.as_writer().await?.flush().await?;
file.as_writer().await?.as_mut().sync_all().await?;
drop(file);
new_local_worker(
Arc::new(LocalWorkerConfig {
work_directory: work_directory.clone(),
Expand Down Expand Up @@ -388,22 +390,24 @@ mod local_worker_tests {
#[cfg(target_family = "unix")]
let precondition_script = {
let precondition_script = format!("{}/precondition.sh", temp_path);
let mut file = fs::create_file(OsString::from(&precondition_script)).await?;
file.as_writer().await?.write_all(b"#!/bin/sh\nexit 1\n").await?;
file.as_writer()
.await?
.as_mut()
.set_permissions(Permissions::from_mode(0o777))
.await?;
file.as_writer().await?.flush().await?;
// We use std::fs::File here because we sometimes get strange bugs here
// that result in: "Text file busy (os error 26)" if it is an executeable.
// It is likley because somewhere the file descriotor does not get closed
// in tokio's async context.
let mut file = std::fs::File::create(OsString::from(&precondition_script))?;
file.write_all(b"#!/bin/sh\nexit 1\n")?;
file.set_permissions(Permissions::from_mode(0o777))?;
file.sync_all()?;
drop(file);
precondition_script
};
#[cfg(target_family = "windows")]
let precondition_script = {
let precondition_script = format!("{}/precondition.bat", temp_path);
let mut file = fs::create_file(OsString::from(&precondition_script)).await?;
file.as_writer().await?.write_all(b"@echo off\r\nexit 1").await?;
file.as_writer().await?.flush().await?;
let mut file = std::fs::File::create(OsString::from(&precondition_script))?;
file.write_all(b"@echo off\r\nexit 1")?;
file.sync_all()?;
drop(file);
precondition_script
};
let local_worker_config = LocalWorkerConfig {
Expand Down
71 changes: 25 additions & 46 deletions cas/worker/tests/running_actions_manager_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::env;
use std::ffi::OsString;
#[cfg(target_family = "unix")]
use std::fs::Permissions;
use std::io::Cursor;
use std::io::{Cursor, Write};
#[cfg(target_family = "unix")]
use std::os::unix::fs::{MetadataExt, PermissionsExt};
use std::pin::Pin;
Expand All @@ -30,7 +30,6 @@ use futures::{FutureExt, TryFutureExt};
use lazy_static::lazy_static;
use prost::Message;
use rand::{thread_rng, Rng};
use tokio::io::AsyncWriteExt;
use tokio::sync::oneshot;

use ac_utils::{compute_digest, get_and_decode_digest, serialize_and_upload_message};
Expand Down Expand Up @@ -1107,35 +1106,25 @@ exit 0
let test_wrapper_script = OsString::from(test_wrapper_dir + "/test_wrapper_script.sh");
#[cfg(target_family = "windows")]
let test_wrapper_script = OsString::from(test_wrapper_dir + "\\test_wrapper_script.bat");
let mut test_wrapper_script_handle = fs::create_file(&test_wrapper_script).await?;
test_wrapper_script_handle
.as_writer()
.await?
.write_all(TEST_WRAPPER_SCRIPT_CONTENT.as_bytes())
.await?;

// We use std::fs::File here because we sometimes get strange bugs here
// that result in: "Text file busy (os error 26)" if it is an executeable.
// It is likley because somewhere the file descriotor does not get closed
// in tokio's async context.
let mut test_wrapper_script_handle = std::fs::File::create(&test_wrapper_script)?;
test_wrapper_script_handle.write_all(TEST_WRAPPER_SCRIPT_CONTENT.as_bytes())?;
#[cfg(target_family = "unix")]
test_wrapper_script_handle
.as_writer()
.await?
.as_mut()
.set_permissions(Permissions::from_mode(0o755))
.await?;
test_wrapper_script_handle.as_writer().await?.flush().await?;
test_wrapper_script_handle
.as_writer()
.await?
.as_mut()
.sync_all()
.await?;
test_wrapper_script_handle.set_permissions(Permissions::from_mode(0o777))?;
test_wrapper_script_handle.sync_all()?;
drop(test_wrapper_script_handle);

test_wrapper_script
};

let mut full_wrapper_script_path = env::current_dir()?;
full_wrapper_script_path.push(test_wrapper_script);
let running_actions_manager = Arc::new(RunningActionsManagerImpl::new(
root_work_directory.clone(),
ExecutionConfiguration {
entrypoint_cmd: Some(full_wrapper_script_path.into_os_string().into_string().unwrap()),
entrypoint_cmd: Some(test_wrapper_script.into_string().unwrap()),
additional_environment: None,
},
Pin::into_inner(cas_store.clone()),
Expand Down Expand Up @@ -1227,35 +1216,25 @@ exit 0
let test_wrapper_script = OsString::from(test_wrapper_dir + "/test_wrapper_script.sh");
#[cfg(target_family = "windows")]
let test_wrapper_script = OsString::from(test_wrapper_dir + "\\test_wrapper_script.bat");
let mut test_wrapper_script_handle = fs::create_file(&test_wrapper_script).await?;
test_wrapper_script_handle
.as_writer()
.await?
.write_all(TEST_WRAPPER_SCRIPT_CONTENT.as_bytes())
.await?;

// We use std::fs::File here because we sometimes get strange bugs here
// that result in: "Text file busy (os error 26)" if it is an executeable.
// It is likley because somewhere the file descriotor does not get closed
// in tokio's async context.
let mut test_wrapper_script_handle = std::fs::File::create(&test_wrapper_script)?;
test_wrapper_script_handle.write_all(TEST_WRAPPER_SCRIPT_CONTENT.as_bytes())?;
#[cfg(target_family = "unix")]
test_wrapper_script_handle
.as_writer()
.await?
.as_mut()
.set_permissions(Permissions::from_mode(0o755))
.await?;
test_wrapper_script_handle.as_writer().await?.flush().await?;
test_wrapper_script_handle
.as_writer()
.await?
.as_mut()
.sync_all()
.await?;
test_wrapper_script_handle.set_permissions(Permissions::from_mode(0o777))?;
test_wrapper_script_handle.sync_all()?;
drop(test_wrapper_script_handle);

test_wrapper_script
};

let mut full_wrapper_script_path = env::current_dir()?;
full_wrapper_script_path.push(test_wrapper_script);
let running_actions_manager = Arc::new(RunningActionsManagerImpl::new(
root_work_directory.clone(),
ExecutionConfiguration {
entrypoint_cmd: Some(full_wrapper_script_path.into_os_string().into_string().unwrap()),
entrypoint_cmd: Some(test_wrapper_script.into_string().unwrap()),
additional_environment: Some(HashMap::from([
(
"PROPERTY".to_string(),
Expand Down
4 changes: 0 additions & 4 deletions util/tests/fs_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ mod fs_tests {
assert_eq!(fs::get_open_files_for_test(), 0);
file.as_writer().await?.write_all(b"Goodbye").await?;
assert_eq!(fs::get_open_files_for_test(), 1);
file.as_writer().await?.flush().await?;
file.as_writer().await?.as_mut().sync_all().await?;
}
assert_eq!(fs::get_open_files_for_test(), 0);
Expand All @@ -75,7 +74,6 @@ mod fs_tests {
{
let mut file = fs::create_file(&filename).await?;
file.as_writer().await?.write_all(DUMMYDATA.as_bytes()).await?;
file.as_writer().await?.flush().await?;
file.as_writer().await?.as_mut().sync_all().await?;
}
{
Expand Down Expand Up @@ -107,7 +105,6 @@ mod fs_tests {
{
let mut file = fs::create_file(&filename).await?;
file.as_writer().await?.write_all(DUMMYDATA.as_bytes()).await?;
file.as_writer().await?.flush().await?;
file.as_writer().await?.as_mut().sync_all().await?;
}
{
Expand Down Expand Up @@ -143,7 +140,6 @@ mod fs_tests {
{
let mut file = fs::create_file(&filename).await?;
file.as_writer().await?.write_all(DUMMYDATA.as_bytes()).await?;
file.as_writer().await?.flush().await?;
file.as_writer().await?.as_mut().sync_all().await?;
}
{
Expand Down

0 comments on commit b730a90

Please sign in to comment.