diff --git a/.github/workflows/demo_test.yml b/.github/workflows/demo_test.yml index 2543d968..ab003a34 100644 --- a/.github/workflows/demo_test.yml +++ b/.github/workflows/demo_test.yml @@ -56,6 +56,9 @@ jobs: run: docker exec language_support_test bash -c "cd /root/occlum/demos/embedded_mode && SGX_MODE=SIM make; SGX_MODE=SIM make test" + - name: Run Golang sqlite test + run: docker exec language_support_test bash -c "cd /root/occlum/demos/golang/go_sqlite/ && SGX_MODE=SIM ./run_go_sqlite_demo.sh" + - name: Go Server set up and run run: docker exec language_support_test bash -c "cd /root/occlum/demos/golang/web_server && occlum-go mod init web_server && occlum-go get -u -v github.com/gin-gonic/gin; occlum-go build -o web_server ./web_server.go; @@ -72,9 +75,6 @@ jobs: sleep ${{ env.nap_time }}; docker exec language_support_test bash -c "cd /root/occlum/demos/golang/grpc_pingpong && SGX_MODE=SIM ./run_ping_on_occlum.sh" & - - name: Run Golang sqlite test - run: docker exec language_support_test bash -c "cd /root/occlum/demos/golang/go_sqlite/ && SGX_MODE=SIM ./run_go_sqlite_demo.sh" - - name: Curl test run: | sleep ${{ env.nap_time }}; diff --git a/.github/workflows/hw_mode_test.yml b/.github/workflows/hw_mode_test.yml index 7d024720..7b38aa16 100644 --- a/.github/workflows/hw_mode_test.yml +++ b/.github/workflows/hw_mode_test.yml @@ -239,6 +239,10 @@ jobs: run: docker exec $language_support_test bash -c "cd /root/occlum/demos/embedded_mode && make; make test" + - name: Run Golang sqlite test + run: docker exec $language_support_test bash -c "export GO111MODULE=on && export GOPROXY=https://goproxy.cn; + cd /root/occlum/demos/golang/go_sqlite/ && ./run_go_sqlite_demo.sh" + - name: Go server set up and run run: docker exec $language_support_test bash -c "export GO111MODULE=on && export GOPROXY=https://goproxy.cn; cd /root/occlum/demos/golang/web_server && occlum-go mod init web_server && occlum-go get -u -v github.com/gin-gonic/gin; @@ -257,10 +261,6 @@ jobs: sleep ${{ env.nap_time }}; docker exec $language_support_test bash -c "cd /root/occlum/demos/golang/grpc_pingpong && ./run_ping_on_occlum.sh" - - name: Run Golang sqlite test - run: docker exec $language_support_test bash -c "export GO111MODULE=on && export GOPROXY=https://goproxy.cn; - cd /root/occlum/demos/golang/go_sqlite/ && ./run_go_sqlite_demo.sh" - # Sleeps longer to make sure the server is up. - name: Curl test run: | diff --git a/deps/sefs b/deps/sefs index 0a17ee1e..23282388 160000 --- a/deps/sefs +++ b/deps/sefs @@ -1 +1 @@ -Subproject commit 0a17ee1ec824181c1cf2db97a05f612e96879645 +Subproject commit 232823888c83c2ad3a29dee219d26252bcda37b1 diff --git a/src/libos/src/fs/file.rs b/src/libos/src/fs/file.rs index 3cd90819..84f27413 100644 --- a/src/libos/src/fs/file.rs +++ b/src/libos/src/fs/file.rs @@ -43,6 +43,10 @@ pub trait File: Debug + Sync + Send + Any { return_op_unsupported_error!("seek") } + fn position(&self) -> Result { + return_op_unsupported_error!("position") + } + fn metadata(&self) -> Result { return_op_unsupported_error!("metadata") } @@ -83,14 +87,16 @@ pub trait File: Debug + Sync + Send + Any { return_op_unsupported_error!("set_status_flags") } - fn test_advisory_lock(&self, lock: &mut Flock) -> Result<()> { + fn test_advisory_lock(&self, lock: &mut RangeLock) -> Result<()> { return_op_unsupported_error!("test_advisory_lock") } - fn set_advisory_lock(&self, lock: &Flock) -> Result<()> { + fn set_advisory_lock(&self, lock: &RangeLock, is_nonblocking: bool) -> Result<()> { return_op_unsupported_error!("set_advisory_lock") } + fn release_advisory_locks(&self) {} + fn fallocate(&self, _flags: FallocateFlags, _offset: usize, _len: usize) -> Result<()> { return_op_unsupported_error!("fallocate") } diff --git a/src/libos/src/fs/file_ops/close.rs b/src/libos/src/fs/file_ops/close.rs index 2dcfe5b5..380d0aef 100644 --- a/src/libos/src/fs/file_ops/close.rs +++ b/src/libos/src/fs/file_ops/close.rs @@ -3,13 +3,6 @@ use super::*; pub fn do_close(fd: FileDesc) -> Result<()> { debug!("close: fd: {}", fd); let current = current!(); - let mut files = current.files().lock().unwrap(); - let file = files.del(fd)?; - // Deadlock note: EpollFile's drop method needs to access file table. So - // if the drop method is invoked inside the del method, then there will be - // a deadlock. - // TODO: make FileTable a struct of internal mutability to avoid deadlock. - drop(files); - drop(file); + current.close_file(fd)?; Ok(()) } diff --git a/src/libos/src/fs/file_ops/fcntl.rs b/src/libos/src/fs/file_ops/fcntl.rs index b1703a4e..1395b0d0 100644 --- a/src/libos/src/fs/file_ops/fcntl.rs +++ b/src/libos/src/fs/file_ops/fcntl.rs @@ -1,4 +1,4 @@ -use super::flock::flock; +use super::flock::c_flock; use super::*; use util::mem_util::from_user; @@ -19,9 +19,11 @@ pub enum FcntlCmd<'a> { /// Set the file status flags SetFl(u32), /// Test a file lock - GetLk(&'a mut flock), + GetLk(&'a mut c_flock), /// Acquire or release a file lock - SetLk(&'a flock), + SetLk(&'a c_flock), + /// The blocking version of SetLK + SetLkWait(&'a c_flock), } impl<'a> FcntlCmd<'a> { @@ -35,16 +37,22 @@ impl<'a> FcntlCmd<'a> { libc::F_GETFL => FcntlCmd::GetFl(), libc::F_SETFL => FcntlCmd::SetFl(arg as u32), libc::F_GETLK => { - let flock_mut_ptr = arg as *mut flock; - from_user::check_mut_ptr(flock_mut_ptr)?; - let flock_mut_c = unsafe { &mut *flock_mut_ptr }; - FcntlCmd::GetLk(flock_mut_c) + let lock_mut_ptr = arg as *mut c_flock; + from_user::check_mut_ptr(lock_mut_ptr)?; + let lock_mut_c = unsafe { &mut *lock_mut_ptr }; + FcntlCmd::GetLk(lock_mut_c) } libc::F_SETLK => { - let flock_ptr = arg as *const flock; - from_user::check_ptr(flock_ptr)?; - let flock_c = unsafe { &*flock_ptr }; - FcntlCmd::SetLk(flock_c) + let lock_ptr = arg as *const c_flock; + from_user::check_ptr(lock_ptr)?; + let lock_c = unsafe { &*lock_ptr }; + FcntlCmd::SetLk(lock_c) + } + libc::F_SETLKW => { + let lock_ptr = arg as *const c_flock; + from_user::check_ptr(lock_ptr)?; + let lock_c = unsafe { &*lock_ptr }; + FcntlCmd::SetLkWait(lock_c) } _ => return_errno!(EINVAL, "unsupported command"), }) @@ -92,20 +100,39 @@ pub fn do_fcntl(fd: FileDesc, cmd: &mut FcntlCmd) -> Result { file.set_status_flags(status_flags)?; 0 } - FcntlCmd::GetLk(flock_mut_c) => { + FcntlCmd::GetLk(lock_mut_c) => { let file = file_table.get(fd)?; - let mut lock = Flock::from_c(*flock_mut_c)?; - if let FlockType::F_UNLCK = lock.l_type { + let lock_type = RangeLockType::from_u16(lock_mut_c.l_type)?; + if RangeLockType::F_UNLCK == lock_type { return_errno!(EINVAL, "invalid flock type for getlk"); } + let mut lock = RangeLockBuilder::new() + .type_(lock_type) + .range(FileRange::from_c_flock_and_file(&lock_mut_c, &file)?) + .build()?; file.test_advisory_lock(&mut lock)?; - (*flock_mut_c).copy_from_safe(&lock); + trace!("getlk returns: {:?}", lock); + (*lock_mut_c).copy_from_range_lock(&lock); 0 } - FcntlCmd::SetLk(flock_c) => { + FcntlCmd::SetLk(lock_c) => { let file = file_table.get(fd)?; - let lock = Flock::from_c(*flock_c)?; - file.set_advisory_lock(&lock)?; + let lock = RangeLockBuilder::new() + .type_(RangeLockType::from_u16(lock_c.l_type)?) + .range(FileRange::from_c_flock_and_file(&lock_c, &file)?) + .build()?; + let is_nonblocking = true; + file.set_advisory_lock(&lock, is_nonblocking)?; + 0 + } + FcntlCmd::SetLkWait(lock_c) => { + let file = file_table.get(fd)?; + let lock = RangeLockBuilder::new() + .type_(RangeLockType::from_u16(lock_c.l_type)?) + .range(FileRange::from_c_flock_and_file(&lock_c, &file)?) + .build()?; + let is_nonblocking = false; + file.set_advisory_lock(&lock, is_nonblocking)?; 0 } }; diff --git a/src/libos/src/fs/file_ops/flock.rs b/src/libos/src/fs/file_ops/flock.rs deleted file mode 100644 index 74b28cb7..00000000 --- a/src/libos/src/fs/file_ops/flock.rs +++ /dev/null @@ -1,88 +0,0 @@ -/// File POSIX advisory lock -use super::*; -use process::pid_t; - -/// C struct for a lock -#[repr(C)] -#[derive(Debug, Copy, Clone)] -pub struct flock { - pub l_type: u16, - pub l_whence: u16, - pub l_start: off_t, - pub l_len: off_t, - pub l_pid: pid_t, -} - -impl flock { - pub fn copy_from_safe(&mut self, lock: &Flock) { - self.l_type = lock.l_type as u16; - self.l_whence = lock.l_whence as u16; - self.l_start = lock.l_start; - self.l_len = lock.l_len; - self.l_pid = lock.l_pid; - } -} - -/// Type safe representation of flock -#[derive(Debug, Copy, Clone)] -pub struct Flock { - pub l_type: FlockType, - pub l_whence: FlockWhence, - pub l_start: off_t, - pub l_len: off_t, - pub l_pid: pid_t, -} - -impl Flock { - pub fn from_c(flock_c: &flock) -> Result { - let l_type = FlockType::from_u16(flock_c.l_type)?; - let l_whence = FlockWhence::from_u16(flock_c.l_whence)?; - Ok(Self { - l_type: l_type, - l_whence: l_whence, - l_start: flock_c.l_start, - l_len: flock_c.l_len, - l_pid: flock_c.l_pid, - }) - } -} - -#[allow(non_camel_case_types)] -#[derive(Debug, Copy, Clone)] -#[repr(u16)] -pub enum FlockType { - F_RDLCK = 0, - F_WRLCK = 1, - F_UNLCK = 2, -} - -impl FlockType { - pub fn from_u16(_type: u16) -> Result { - Ok(match _type { - 0 => FlockType::F_RDLCK, - 1 => FlockType::F_WRLCK, - 2 => FlockType::F_UNLCK, - _ => return_errno!(EINVAL, "invalid flock type"), - }) - } -} - -#[allow(non_camel_case_types)] -#[derive(Debug, Copy, Clone)] -#[repr(u16)] -pub enum FlockWhence { - SEEK_SET = 0, - SEEK_CUR = 1, - SEEK_END = 2, -} - -impl FlockWhence { - pub fn from_u16(whence: u16) -> Result { - Ok(match whence { - 0 => FlockWhence::SEEK_SET, - 1 => FlockWhence::SEEK_CUR, - 2 => FlockWhence::SEEK_END, - _ => return_errno!(EINVAL, "Invalid whence"), - }) - } -} diff --git a/src/libos/src/fs/file_ops/mod.rs b/src/libos/src/fs/file_ops/mod.rs index 8efeb4a9..e949cee7 100644 --- a/src/libos/src/fs/file_ops/mod.rs +++ b/src/libos/src/fs/file_ops/mod.rs @@ -9,7 +9,6 @@ pub use self::dup::{do_dup, do_dup2, do_dup3}; pub use self::fallocate::{do_fallocate, FallocateFlags}; pub use self::fcntl::{do_fcntl, FcntlCmd}; pub use self::file_flags::{AccessMode, CreationFlags, StatusFlags}; -pub use self::flock::{Flock, FlockType}; pub use self::fspath::{get_abs_path_by_fd, FsPath, AT_FDCWD}; pub use self::fsync::{do_fdatasync, do_fsync}; pub use self::getdents::{do_getdents, do_getdents64}; @@ -39,7 +38,6 @@ mod dup; mod fallocate; mod fcntl; mod file_flags; -mod flock; mod fspath; mod fsync; mod getdents; diff --git a/src/libos/src/fs/file_table.rs b/src/libos/src/fs/file_table.rs index d719ad5c..75de0cee 100644 --- a/src/libos/src/fs/file_table.rs +++ b/src/libos/src/fs/file_table.rs @@ -151,9 +151,31 @@ impl FileTable { } } - /// Remove file descriptors that are close-on-spawn - pub fn close_on_spawn(&mut self) { + /// Remove all the file descriptors + pub fn del_all(&mut self) -> Vec { let mut deleted_fds = Vec::new(); + let mut deleted_files = Vec::new(); + for (fd, entry) in self + .table + .iter_mut() + .filter(|entry| entry.is_some()) + .enumerate() + { + deleted_files.push(entry.as_ref().unwrap().file.clone()); + *entry = None; + deleted_fds.push(fd as FileDesc); + } + self.num_fds = 0; + for fd in deleted_fds { + self.broadcast_del(fd); + } + deleted_files + } + + /// Remove file descriptors that are close-on-spawn + pub fn close_on_spawn(&mut self) -> Vec { + let mut deleted_fds = Vec::new(); + let mut deleted_files = Vec::new(); for (fd, entry) in self.table.iter_mut().enumerate() { let need_close = if let Some(entry) = entry { entry.close_on_spawn @@ -161,6 +183,7 @@ impl FileTable { false }; if need_close { + deleted_files.push(entry.as_ref().unwrap().file.clone()); *entry = None; deleted_fds.push(fd as FileDesc); self.num_fds -= 1; @@ -170,6 +193,7 @@ impl FileTable { for fd in deleted_fds { self.broadcast_del(fd); } + deleted_files } pub fn notifier(&self) -> &FileTableNotifier { diff --git a/src/libos/src/fs/flock/builder.rs b/src/libos/src/fs/flock/builder.rs new file mode 100644 index 00000000..94d45683 --- /dev/null +++ b/src/libos/src/fs/flock/builder.rs @@ -0,0 +1,58 @@ +use super::*; + +pub struct RangeLockBuilder { + // Mandatory field + type_: Option, + range: Option, + // Optional fields + owner: Option, + waiters: Option, +} + +impl RangeLockBuilder { + pub fn new() -> Self { + Self { + owner: None, + type_: None, + range: None, + waiters: None, + } + } + + pub fn owner(mut self, owner: pid_t) -> Self { + self.owner = Some(owner); + self + } + + pub fn type_(mut self, type_: RangeLockType) -> Self { + self.type_ = Some(type_); + self + } + + pub fn range(mut self, range: FileRange) -> Self { + self.range = Some(range); + self + } + + pub fn waiters(mut self, waiters: WaiterQueue) -> Self { + self.waiters = Some(waiters); + self + } + + pub fn build(self) -> Result { + let owner = self.owner.unwrap_or_else(|| current!().process().pid()); + let type_ = self + .type_ + .ok_or_else(|| errno!(EINVAL, "type_ is mandatory"))?; + let range = self + .range + .ok_or_else(|| errno!(EINVAL, "range is mandatory"))?; + let waiters = self.waiters; + Ok(RangeLock { + owner, + type_, + range, + waiters, + }) + } +} diff --git a/src/libos/src/fs/flock/mod.rs b/src/libos/src/fs/flock/mod.rs new file mode 100644 index 00000000..0007c0e7 --- /dev/null +++ b/src/libos/src/fs/flock/mod.rs @@ -0,0 +1,387 @@ +/// File POSIX advisory range locks +use super::*; +use crate::events::{Waiter, WaiterQueue}; +use crate::util::sync::rw_lock::RwLockWriteGuard; +use process::pid_t; +use rcore_fs::vfs::AnyExt; + +pub use self::builder::RangeLockBuilder; +pub use self::range::{FileRange, OverlapWith, OFFSET_MAX}; +use self::range::{FileRangeChange, RangeLockWhence}; + +mod builder; +mod range; + +/// C struct for a file range lock in Libc +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct c_flock { + /// Type of lock: F_RDLCK, F_WRLCK, or F_UNLCK + pub l_type: u16, + /// Where `l_start' is relative to + pub l_whence: u16, + /// Offset where the lock begins + pub l_start: off_t, + /// Size of the locked area, 0 means until EOF + pub l_len: off_t, + /// Process holding the lock + pub l_pid: pid_t, +} + +impl c_flock { + pub fn copy_from_range_lock(&mut self, lock: &RangeLock) { + self.l_type = lock.type_ as u16; + if RangeLockType::F_UNLCK != lock.type_ { + self.l_whence = RangeLockWhence::SEEK_SET as u16; + self.l_start = lock.start() as off_t; + self.l_len = if lock.end() == OFFSET_MAX { + 0 + } else { + lock.range.len() as off_t + }; + self.l_pid = lock.owner; + } + } +} + +/// Kernel representation of file range lock +pub struct RangeLock { + /// Owner of lock, process holding the lock + owner: pid_t, + /// Type of lock, F_RDLCK, F_WRLCK, or F_UNLCK + type_: RangeLockType, + /// Range of lock + range: FileRange, + /// Optional waiters that are blocking by the lock + waiters: Option, +} + +impl RangeLock { + pub fn type_(&self) -> RangeLockType { + self.type_ + } + + pub fn set_type(&mut self, type_: RangeLockType) { + self.type_ = type_; + } + + pub fn owner(&self) -> pid_t { + self.owner + } + + pub fn conflict_with(&self, other: &Self) -> bool { + // locks owned by the same process do not conflict + if self.owner == other.owner { + return false; + } + // locks do not conflict if not overlap + if self.overlap_with(other).is_none() { + return false; + } + // write lock is exclusive + if self.type_ == RangeLockType::F_WRLCK || other.type_ == RangeLockType::F_WRLCK { + return true; + } + false + } + + pub fn overlap_with(&self, other: &Self) -> Option { + self.range.overlap_with(&other.range) + } + + pub fn merge_with(&mut self, other: &Self) { + self.range.merge(&other.range).expect("merge range failed"); + } + + pub fn start(&self) -> usize { + self.range.start() + } + + pub fn end(&self) -> usize { + self.range.end() + } + + pub fn set_start(&mut self, new_start: usize) { + let change = self.range.set_start(new_start).expect("invalid new start"); + if let FileRangeChange::Shrinked = change { + self.dequeue_and_wake_all_waiters(); + } + } + + pub fn set_end(&mut self, new_end: usize) { + let change = self.range.set_end(new_end).expect("invalid new end"); + if let FileRangeChange::Shrinked = change { + self.dequeue_and_wake_all_waiters(); + } + } + + pub fn enqueue_waiter(&mut self, waiter: &Waiter) { + if self.waiters.is_none() { + self.waiters = Some(WaiterQueue::new()); + } + self.waiters.as_ref().unwrap().reset_and_enqueue(waiter) + } + + pub fn dequeue_and_wake_all_waiters(&mut self) -> usize { + if self.waiters.is_some() { + return self.waiters.as_ref().unwrap().dequeue_and_wake_all(); + } + 0 + } +} + +impl Drop for RangeLock { + fn drop(&mut self) { + self.dequeue_and_wake_all_waiters(); + } +} + +impl Debug for RangeLock { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("RangeLock") + .field("owner", &self.owner) + .field("type_", &self.type_) + .field("range", &self.range) + .finish() + } +} + +impl Clone for RangeLock { + fn clone(&self) -> Self { + Self { + owner: self.owner.clone(), + type_: self.type_.clone(), + range: self.range.clone(), + waiters: None, + } + } +} + +/// List of File POSIX advisory range locks. +/// +/// Rule of ordering: +/// Locks are sorted by owner process, then by the starting offset. +/// +/// Rule of mergeing: +/// Adjacent and overlapping locks with same owner and type will be merged. +/// +/// Rule of updating: +/// New locks with different type will replace or split the overlapping locks +/// if they have same owner. +/// +pub struct RangeLockList { + inner: RwLock>, +} + +impl RangeLockList { + pub fn new() -> Self { + Self { + inner: RwLock::new(VecDeque::new()), + } + } + + pub fn test_lock(&self, lock: &mut RangeLock) { + debug!("test_lock with RangeLock: {:?}", lock); + let list = self.inner.read().unwrap(); + for existing_lock in list.iter() { + if lock.conflict_with(existing_lock) { + // Return the information about the conflict lock + lock.owner = existing_lock.owner; + lock.type_ = existing_lock.type_; + lock.range = existing_lock.range; + return; + } + } + // The lock could be placed at this time + lock.type_ = RangeLockType::F_UNLCK; + } + + pub fn set_lock(&self, lock: &RangeLock, is_nonblocking: bool) -> Result<()> { + debug!( + "set_lock with RangeLock: {:?}, is_nonblocking: {}", + lock, is_nonblocking + ); + loop { + let mut list = self.inner.write().unwrap(); + if let Some(mut conflict_lock) = list.iter_mut().find(|l| l.conflict_with(lock)) { + if is_nonblocking { + return_errno!(EAGAIN, "lock conflict, try again later"); + } + // Start to wait + let waiter = Waiter::new(); + // TODO: Add deadlock detection, and returns EDEADLK + warn!("Do not support deadlock detection, maybe wait infinitely"); + conflict_lock.enqueue_waiter(&waiter); + // Ensure that we drop any locks before wait + drop(list); + waiter.wait(None)?; + // Wake up, let's try to set lock again + continue; + } + // No conflict here, let's insert the lock + return Ok(Self::insert_lock_into_list(&mut list, lock)); + } + } + + fn insert_lock_into_list(list: &mut RwLockWriteGuard>, lock: &RangeLock) { + let first_same_owner_idx = match list.iter().position(|lk| lk.owner() == lock.owner()) { + Some(idx) => idx, + None => { + // Can't find existing locks with same owner. + list.push_front(lock.clone()); + return; + } + }; + // Insert the lock at the start position with same owner, may breaking + // the rules of RangeLockList. + // We will handle the inserted lock with next one to adjust the list to + // obey the rules. + list.insert(first_same_owner_idx, lock.clone()); + let mut pre_idx = first_same_owner_idx; + let mut next_idx = pre_idx + 1; + loop { + if next_idx >= list.len() { + break; + } + let pre_lock = list[pre_idx].clone(); + let next_lock = list[next_idx].clone(); + + if next_lock.owner() != pre_lock.owner() { + break; + } + if next_lock.type_() == pre_lock.type_() { + // Same type + if pre_lock.end() < next_lock.start() { + break; + } else if next_lock.end() < pre_lock.start() { + list.swap(pre_idx, next_idx); + pre_idx += 1; + next_idx += 1; + } else { + // Merge adjacent or overlapping locks + list[next_idx].merge_with(&pre_lock); + list.remove(pre_idx); + } + } else { + // Different type + if pre_lock.end() <= next_lock.start() { + break; + } else if next_lock.end() <= pre_lock.start() { + list.swap(pre_idx, next_idx); + pre_idx += 1; + next_idx += 1; + } else { + // Split overlapping locks + let overlap_with = pre_lock.overlap_with(&next_lock).unwrap(); + match overlap_with { + OverlapWith::ToLeft => { + list[next_idx].set_start(pre_lock.end()); + break; + } + OverlapWith::InMiddle => { + let right_lk = { + let mut r_lk = next_lock.clone(); + r_lk.set_start(pre_lock.end()); + r_lk + }; + list[next_idx].set_end(pre_lock.start()); + list.swap(pre_idx, next_idx); + list.insert(next_idx + 1, right_lk); + break; + } + OverlapWith::ToRight => { + list[next_idx].set_end(pre_lock.start()); + list.swap(pre_idx, next_idx); + pre_idx += 1; + next_idx += 1; + } + OverlapWith::Includes => { + // New lock can replace the old one + list.remove(next_idx); + } + } + } + } + } + } + + pub fn unlock(&self, lock: &RangeLock) { + debug!("unlock with RangeLock: {:?}", lock); + let mut list = self.inner.write().unwrap(); + let mut skipped = 0; + loop { + let idx = match list + .iter() + .skip(skipped) + .position(|lk| lk.owner() == lock.owner()) + { + Some(idx) => idx, + None => break, + }; + + let existing_lock = &mut list[idx]; + let overlap_with = match lock.overlap_with(existing_lock) { + Some(overlap_with) => overlap_with, + None => { + skipped = idx + 1; + continue; + } + }; + + match overlap_with { + OverlapWith::ToLeft => { + existing_lock.set_start(lock.end()); + break; + } + OverlapWith::InMiddle => { + // Split the lock + let right_lk = { + let mut r_lk = existing_lock.clone(); + r_lk.set_start(lock.end()); + r_lk + }; + existing_lock.set_end(lock.start()); + list.insert(idx + 1, right_lk); + break; + } + OverlapWith::ToRight => { + existing_lock.set_end(lock.start()); + skipped = idx + 1; + } + OverlapWith::Includes => { + // The lock can be deleted from the list + list.remove(idx); + skipped = idx; + } + } + } + } +} + +impl Default for RangeLockList { + fn default() -> Self { + Self::new() + } +} + +impl AnyExt for RangeLockList {} + +#[allow(non_camel_case_types)] +#[derive(Debug, Copy, Clone, PartialEq)] +#[repr(u16)] +pub enum RangeLockType { + F_RDLCK = 0, + F_WRLCK = 1, + F_UNLCK = 2, +} + +impl RangeLockType { + pub fn from_u16(_type: u16) -> Result { + Ok(match _type { + 0 => RangeLockType::F_RDLCK, + 1 => RangeLockType::F_WRLCK, + 2 => RangeLockType::F_UNLCK, + _ => return_errno!(EINVAL, "invalid flock type"), + }) + } +} diff --git a/src/libos/src/fs/flock/range.rs b/src/libos/src/fs/flock/range.rs new file mode 100644 index 00000000..eec17668 --- /dev/null +++ b/src/libos/src/fs/flock/range.rs @@ -0,0 +1,175 @@ +use super::*; + +pub const OFFSET_MAX: usize = off_t::MAX as usize; + +#[derive(Debug, Copy, Clone)] +pub struct FileRange { + start: usize, + end: usize, +} + +impl FileRange { + /// Create the range through C flock and opened file reference + pub fn from_c_flock_and_file(lock: &c_flock, file: &FileRef) -> Result { + let start = { + let whence = RangeLockWhence::from_u16(lock.l_whence)?; + match whence { + RangeLockWhence::SEEK_SET => lock.l_start, + RangeLockWhence::SEEK_CUR => file + .position()? + .checked_add(lock.l_start) + .ok_or_else(|| errno!(EOVERFLOW, "start overflow"))?, + RangeLockWhence::SEEK_END => (file.metadata()?.size as off_t) + .checked_add(lock.l_start) + .ok_or_else(|| errno!(EOVERFLOW, "start overflow"))?, + } + }; + if start < 0 { + return_errno!(EINVAL, "invalid start"); + } + + let (start, end) = if lock.l_len > 0 { + let end = start + .checked_add(lock.l_len) + .ok_or_else(|| errno!(EOVERFLOW, "end overflow"))?; + (start as usize, end as usize) + } else if lock.l_len == 0 { + (start as usize, OFFSET_MAX) + } else { + // len < 0, must recalculate the start + let end = start; + let new_start = start + lock.l_len; + if new_start < 0 { + return_errno!(EINVAL, "invalid len"); + } + (new_start as usize, end as usize) + }; + + Ok(Self { start, end }) + } + + pub fn new(start: usize, end: usize) -> Result { + if start >= end { + return_errno!(EINVAL, "invalid parameters"); + } + Ok(Self { start, end }) + } + + pub fn len(&self) -> usize { + self.end - self.start + } + + pub fn start(&self) -> usize { + self.start + } + + pub fn end(&self) -> usize { + self.end + } + + pub fn set_start(&mut self, new_start: usize) -> Result { + if new_start >= self.end { + return_errno!(EINVAL, "invalid new start"); + } + let old_start = self.start; + self.start = new_start; + let change = if new_start > old_start { + FileRangeChange::Shrinked + } else if new_start < old_start { + FileRangeChange::Expanded + } else { + FileRangeChange::Same + }; + Ok(change) + } + + pub fn set_end(&mut self, new_end: usize) -> Result { + if new_end <= self.start { + return_errno!(EINVAL, "invalid new end"); + } + let old_end = self.end; + self.end = new_end; + let change = if new_end < old_end { + FileRangeChange::Shrinked + } else if new_end > old_end { + FileRangeChange::Expanded + } else { + FileRangeChange::Same + }; + Ok(change) + } + + pub fn overlap_with(&self, other: &Self) -> Option { + if self.start >= other.end || self.end <= other.start { + return None; + } + + let overlap = if self.start <= other.start && self.end < other.end { + OverlapWith::ToLeft + } else if self.start > other.start && self.end < other.end { + OverlapWith::InMiddle + } else if self.start > other.start && self.end >= other.end { + OverlapWith::ToRight + } else { + OverlapWith::Includes + }; + Some(overlap) + } + + pub fn merge(&mut self, other: &Self) -> Result { + if self.end < other.start || other.end < self.start { + return_errno!(EINVAL, "can not merge separated ranges"); + } + + let mut change = FileRangeChange::Same; + if other.start < self.start { + self.start = other.start; + change = FileRangeChange::Expanded; + } + if other.end > self.end { + self.end = other.end; + change = FileRangeChange::Expanded; + } + Ok(change) + } +} + +#[derive(Debug)] +pub enum FileRangeChange { + Same, + Expanded, + Shrinked, +} + +/// The position of a range (say A) relative another overlapping range (say B). +#[derive(Debug)] +pub enum OverlapWith { + /// The position where range A is to the left of B (A.start <= B.start && A.end < B.end). + ToLeft, + /// The position where range A is to the right of B (A.start > B.start && A.end >= B.end). + ToRight, + /// The position where range A is in the middle of B (A.start > B.start && A.end < B.end). + InMiddle, + /// The position where range A includes B (A.start <= B.start && A.end >= B.end). + Includes, +} + +#[allow(non_camel_case_types)] +#[derive(Debug, Copy, Clone)] +#[repr(u16)] +pub enum RangeLockWhence { + SEEK_SET = 0, + SEEK_CUR = 1, + SEEK_END = 2, +} + +impl RangeLockWhence { + pub fn from_u16(whence: u16) -> Result { + Ok(match whence { + 0 => RangeLockWhence::SEEK_SET, + 1 => RangeLockWhence::SEEK_CUR, + 2 => RangeLockWhence::SEEK_END, + _ => return_errno!(EINVAL, "Invalid whence"), + }) + } +} diff --git a/src/libos/src/fs/inode_file.rs b/src/libos/src/fs/inode_file.rs index ad6a122a..1ede1130 100644 --- a/src/libos/src/fs/inode_file.rs +++ b/src/libos/src/fs/inode_file.rs @@ -112,6 +112,11 @@ impl File for INodeFile { Ok(*offset as i64) } + fn position(&self) -> Result { + let offset = self.offset.lock().unwrap(); + Ok(*offset as off_t) + } + fn metadata(&self) -> Result { let metadata = self.inode.metadata()?; Ok(metadata) @@ -182,30 +187,60 @@ impl File for INodeFile { Ok(()) } - fn test_advisory_lock(&self, lock: &mut Flock) -> Result<()> { - // Let the advisory lock could be placed - // TODO: Implement the real advisory lock - lock.l_type = FlockType::F_UNLCK; + fn test_advisory_lock(&self, lock: &mut RangeLock) -> Result<()> { + let ext = match self.inode.ext() { + Some(ext) => ext, + None => { + warn!("Inode extension is not supportted, the lock could be placed"); + lock.set_type(RangeLockType::F_UNLCK); + return Ok(()); + } + }; + + match ext.get::() { + None => { + // The advisory lock could be placed if there is no lock list + lock.set_type(RangeLockType::F_UNLCK); + } + Some(range_lock_list) => { + range_lock_list.test_lock(lock); + } + } Ok(()) } - fn set_advisory_lock(&self, lock: &Flock) -> Result<()> { - match lock.l_type { - FlockType::F_RDLCK => { - if !self.access_mode.readable() { - return_errno!(EACCES, "File not readable"); - } - } - FlockType::F_WRLCK => { - if !self.access_mode.writable() { - return_errno!(EACCES, "File not writable"); - } - } - _ => (), + fn set_advisory_lock(&self, lock: &RangeLock, is_nonblocking: bool) -> Result<()> { + if RangeLockType::F_UNLCK == lock.type_() { + return Ok(self.unlock_range_lock(lock)); } - // Let the advisory lock could be acquired or released - // TODO: Implement the real advisory lock - Ok(()) + + self.check_advisory_lock_with_access_mode(lock)?; + let ext = match self.inode.ext() { + Some(ext) => ext, + None => { + warn!( + "Inode extension is not supported, let the lock could be acquired or released" + ); + // TODO: Implement inode extension for FS + return Ok(()); + } + }; + let range_lock_list = match ext.get::() { + Some(list) => list, + None => ext.get_or_put_default::(), + }; + + range_lock_list.set_lock(lock, is_nonblocking) + } + + fn release_advisory_locks(&self) { + let range_lock = RangeLockBuilder::new() + .type_(RangeLockType::F_UNLCK) + .range(FileRange::new(0, OFFSET_MAX).unwrap()) + .build() + .unwrap(); + + self.unlock_range_lock(&range_lock) } fn ioctl(&self, cmd: &mut IoctlCmd) -> Result { @@ -269,6 +304,40 @@ impl INodeFile { pub fn abs_path(&self) -> &str { &self.abs_path } + + fn check_advisory_lock_with_access_mode(&self, lock: &RangeLock) -> Result<()> { + match lock.type_() { + RangeLockType::F_RDLCK => { + if !self.access_mode.readable() { + return_errno!(EBADF, "File not readable"); + } + } + RangeLockType::F_WRLCK => { + if !self.access_mode.writable() { + return_errno!(EBADF, "File not writable"); + } + } + _ => (), + } + Ok(()) + } + + fn unlock_range_lock(&self, lock: &RangeLock) { + let ext = match self.inode.ext() { + Some(ext) => ext, + None => { + return; + } + }; + let range_lock_list = match ext.get::() { + Some(list) => list, + None => { + return; + } + }; + + range_lock_list.unlock(lock) + } } impl Debug for INodeFile { diff --git a/src/libos/src/fs/mod.rs b/src/libos/src/fs/mod.rs index 2659d1e5..ff38e4f3 100644 --- a/src/libos/src/fs/mod.rs +++ b/src/libos/src/fs/mod.rs @@ -20,10 +20,12 @@ pub use self::events::{AtomicIoEvents, IoEvents, IoNotifier}; pub use self::file::{File, FileRef}; pub use self::file_ops::{ occlum_ocall_ioctl, AccessMode, BuiltinIoctlNum, CreationFlags, FallocateFlags, FileMode, - Flock, FlockType, IfConf, IoctlCmd, Stat, StatusFlags, StructuredIoctlArgType, - StructuredIoctlNum, + IfConf, IoctlCmd, Stat, StatusFlags, StructuredIoctlArgType, StructuredIoctlNum, }; pub use self::file_table::{FileDesc, FileTable, FileTableEvent, FileTableNotifier}; +pub use self::flock::{ + FileRange, RangeLock, RangeLockBuilder, RangeLockList, RangeLockType, OFFSET_MAX, +}; pub use self::fs_ops::Statfs; pub use self::fs_view::FsView; pub use self::host_fd::HostFd; @@ -41,6 +43,7 @@ mod events; mod file; mod file_ops; mod file_table; +mod flock; mod fs_ops; mod fs_view; mod host_fd; diff --git a/src/libos/src/process/do_exit.rs b/src/libos/src/process/do_exit.rs index d0b68ea0..ea8384eb 100644 --- a/src/libos/src/process/do_exit.rs +++ b/src/libos/src/process/do_exit.rs @@ -64,8 +64,9 @@ fn exit_thread(term_status: TermStatus) { table::del_thread(thread.tid()).expect("tid must be in the table"); } - // If this thread is the last thread, then exit the process + // If this thread is the last thread, close all files then exit the process if num_remaining_threads == 0 { + thread.close_all_files(); exit_process(&thread, term_status); } diff --git a/src/libos/src/process/do_spawn/mod.rs b/src/libos/src/process/do_spawn/mod.rs index 13889d58..5bea9812 100644 --- a/src/libos/src/process/do_spawn/mod.rs +++ b/src/libos/src/process/do_spawn/mod.rs @@ -246,7 +246,7 @@ fn new_process_common( }; let vm_ref = Arc::new(vm); let files_ref = { - let files = init_files(current_ref, file_actions, host_stdio_fds)?; + let files = init_files(current_ref, file_actions, host_stdio_fds, &reuse_tid)?; Arc::new(SgxMutex::new(files)) }; let fs_ref = Arc::new(RwLock::new(current_ref.fs().read().unwrap().clone())); @@ -347,12 +347,25 @@ fn init_files( current_ref: &ThreadRef, file_actions: &[FileAction], host_stdio_fds: Option<&HostStdioFds>, + reuse_tid: &Option, ) -> Result { // Usually, we just inherit the file table from the current process let should_inherit_file_table = current_ref.process().pid() > 0; if should_inherit_file_table { // Fork: clone file table let mut cloned_file_table = current_ref.files().lock().unwrap().clone(); + + // By default, file descriptors remain open across an execve(). + // File descriptors that are marked close-on-exec are closed, which will cause + // the release of advisory locks owned by current process. + if reuse_tid.is_some() { + let closed_files = cloned_file_table.close_on_spawn(); + for file in closed_files { + file.release_advisory_locks(); + } + return Ok(cloned_file_table); + } + // Perform file actions to modify the cloned file table for file_action in file_actions { match file_action { diff --git a/src/libos/src/process/thread/mod.rs b/src/libos/src/process/thread/mod.rs index 26ca2133..3d04a246 100644 --- a/src/libos/src/process/thread/mod.rs +++ b/src/libos/src/process/thread/mod.rs @@ -124,6 +124,27 @@ impl Thread { self.files().lock().unwrap().put(new_file, close_on_spawn) } + /// Close a file from the file table. It will release the POSIX advisory locks owned + /// by current process. + pub fn close_file(&self, fd: FileDesc) -> Result<()> { + // Deadlock note: EpollFile's drop method needs to access file table. So + // if the drop method is invoked inside the del method, then there will be + // a deadlock. + let file = self.files().lock().unwrap().del(fd)?; + file.release_advisory_locks(); + Ok(()) + } + + /// Close all files in the file table. It will release the POSIX advisory locks owned + /// by current process. + pub fn close_all_files(&self) { + // Deadlock note: Same with the issue in close_file method + let files = self.files().lock().unwrap().del_all(); + for file in files { + file.release_advisory_locks(); + } + } + pub fn fs(&self) -> &FsViewRef { &self.fs } diff --git a/test/Makefile b/test/Makefile index d169f027..328bf347 100644 --- a/test/Makefile +++ b/test/Makefile @@ -16,8 +16,8 @@ FAIL_LOG = $(BUILD_DIR)/test/.fail TEST_DEPS := client data_sink naughty_child # Tests: need to be compiled and run by test-% target TESTS ?= env empty hello_world malloc mmap file fs_perms getpid spawn sched pipe time timerfd \ - truncate readdir mkdir open stat link symlink chmod chown tls pthread system_info resolv_conf rlimit \ - server server_epoll unix_socket cout hostfs cpuid rdtsc device sleep exit_group \ + truncate readdir mkdir open stat link symlink chmod chown tls pthread system_info rlimit \ + server server_epoll unix_socket cout hostfs cpuid rdtsc device sleep exit_group flock \ ioctl fcntl eventfd emulate_syscall access signal sysinfo prctl rename procfs wait \ spawn_attribute exec statfs random umask pgrp vfork # Benchmarks: need to be compiled and run by bench-% target diff --git a/test/env/main.c b/test/env/main.c index 9270b432..bede0e78 100644 --- a/test/env/main.c +++ b/test/env/main.c @@ -144,7 +144,7 @@ static int test_env_set_child_env_and_argv() { if (ret < 0) { THROW_ERROR("failed to wait4 the child process"); } - if (!WIFEXITED(status)) { + if (!(WIFEXITED(status) && WEXITSTATUS(status) == 0)) { THROW_ERROR("test cases in child faild"); } return 0; diff --git a/test/fcntl/main.c b/test/fcntl/main.c index 4374734c..8b4ad1a8 100644 --- a/test/fcntl/main.c +++ b/test/fcntl/main.c @@ -47,33 +47,6 @@ static int __fcntl_setfl(int fd, int open_flags) { return 0; } -static int __fcntl_getlk_and_setlk(int fd, int open_flags) { - int ret; - struct flock fl = { F_WRLCK, SEEK_SET, 0, 0, 0 }; - - // getlk - ret = fcntl(fd, F_GETLK, &fl); - if (ret < 0) { - THROW_ERROR("failed to call getlk"); - } - if (fl.l_type != F_UNLCK) { - THROW_ERROR("failed to get correct fl type"); - } - - // setlk - if ((open_flags & O_WRONLY) || (open_flags & O_RDWR)) { - fl.l_type = F_WRLCK; - } else { - fl.l_type = F_RDLCK; - } - ret = fcntl(fd, F_SETLK, &fl); - if (ret < 0) { - THROW_ERROR("failed to call setlk"); - } - - return 0; -} - static int __fcntl_dupfd(int fd, int open_flags) { if (fcntl(fd, F_DUPFD, 0) < 0) { THROW_ERROR("failed to duplicate the fd"); @@ -113,10 +86,6 @@ static int test_fcntl_setfl() { return test_fcntl_framework(__fcntl_setfl); } -static int test_getlk_and_setlk() { - return test_fcntl_framework(__fcntl_getlk_and_setlk); -} - static int test_fcntl_dupfd() { return test_fcntl_framework(__fcntl_dupfd); } @@ -128,7 +97,6 @@ static int test_fcntl_dupfd() { static test_case_t test_cases[] = { TEST_CASE(test_fcntl_getfl), TEST_CASE(test_fcntl_setfl), - TEST_CASE(test_getlk_and_setlk), TEST_CASE(test_fcntl_dupfd), }; diff --git a/test/flock/Makefile b/test/flock/Makefile new file mode 100644 index 00000000..9e1b6dec --- /dev/null +++ b/test/flock/Makefile @@ -0,0 +1,5 @@ +include ../test_common.mk + +EXTRA_C_FLAGS := +EXTRA_LINK_FLAGS := +BIN_ARGS := diff --git a/test/flock/main.c b/test/flock/main.c new file mode 100644 index 00000000..b74cd89c --- /dev/null +++ b/test/flock/main.c @@ -0,0 +1,208 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include "test.h" + +// ============================================================================ +// Helper structs & variables & functions +// ============================================================================ + +const char **g_argv; +int g_argc; +const char *g_file_path = "/root/test_flock_file.txt"; +int g_fd; +off_t g_file_len = 128; + +// Expected child arguments +const int child_argc = 2; +const char *child_argv[3] = { + "flock", + "child", + NULL +}; + +static int open_or_create_file() { + int flags = O_RDWR | O_CREAT; + int mode = 00666; + + int fd = open(g_file_path, flags, mode); + if (fd < 0) { + THROW_ERROR("failed to open or create file"); + } + + if (ftruncate(fd, g_file_len) < 0) { + THROW_ERROR("failed to expand the file len"); + } + return fd; +} + +static int remove_file() { + if (unlink(g_file_path) < 0) { + THROW_ERROR("failed to unlink the created file"); + } + return 0; +} + +// ============================================================================ +// Test cases for file POSIX advisory lock +// ============================================================================ + +static int test_getlk() { + struct flock fl = { F_RDLCK, SEEK_SET, 0, 0, 0 }; + if (fcntl(g_fd, F_GETLK, &fl) < 0) { + THROW_ERROR("failed to call getlk"); + } + if (fl.l_type != F_UNLCK) { + THROW_ERROR("failed to get correct fl type"); + } + return 0; +} + +static int test_setlk() { + struct flock fl = { F_RDLCK, SEEK_SET, 0, g_file_len / 2, 0 }; + if (fcntl(g_fd, F_SETLK, &fl) < 0) { + THROW_ERROR("failed to call setlk"); + } + + fl.l_len = g_file_len; + if (fcntl(g_fd, F_SETLK, &fl) < 0) { + THROW_ERROR("failed to expand the lock"); + } + + fl.l_type = F_WRLCK; + fl.l_len = g_file_len / 2; + if (fcntl(g_fd, F_SETLK, &fl) < 0) { + THROW_ERROR("failed change the lock type of existing lock"); + } + + return 0; +} + +static int test_spawn_child_and_unlock() { + int status, child_pid; + int ret = posix_spawn(&child_pid, + "/bin/flock", NULL, NULL, + (char *const *)child_argv, + NULL); + if (ret < 0) { + THROW_ERROR("spawn process error"); + } + printf("Spawn a child process with pid=%d\n", child_pid); + + // Sleep 3s for the child to run setlkw test and wait, is 3s enough? + sleep(3); + + // Unlock the flock will cause child process to finish running + struct flock fl = { F_UNLCK, SEEK_SET, 0, 0, 0 }; + if (fcntl(g_fd, F_SETLK, &fl) < 0) { + THROW_ERROR("failed to unlock the lock"); + } + + // Wait for child exit + ret = wait4(child_pid, &status, 0, NULL); + if (ret < 0) { + THROW_ERROR("failed to wait4 the child process"); + } + if (!(WIFEXITED(status) && WEXITSTATUS(status) == 0)) { + THROW_ERROR("test cases in child faild"); + } + + // The lock will be unlocked on child exit, so we can lock again + struct flock fl2 = { F_WRLCK, SEEK_SET, 0, g_file_len / 4, 0 }; + ret = fcntl(g_fd, F_SETLKW, &fl2); + if (ret < 0 && errno != EINTR) { + THROW_ERROR("failed to check the result of setlkw"); + } + + return 0; +} + +// ============================================================================ +// Child Test cases +// ============================================================================ + +static int test_child_getlk() { + struct flock fl = { F_RDLCK, SEEK_SET, 0, g_file_len / 4, 0 }; + if (fcntl(g_fd, F_GETLK, &fl) < 0) { + THROW_ERROR("failed to call getlk"); + } + + if (fl.l_type != F_WRLCK) { + THROW_ERROR("failed to get correct fl type"); + } + if (fl.l_pid == 0) { + THROW_ERROR("failed to get correct fl pid"); + } + if (fl.l_len != g_file_len / 2) { + THROW_ERROR("failed to get correct fl len"); + } + + return 0; +} + +static int test_child_setlk() { + struct flock fl = { F_RDLCK, SEEK_SET, 0, g_file_len / 4, 0 }; + int res = fcntl(g_fd, F_SETLK, &fl); + if (!(res < 0 && errno == EAGAIN)) { + THROW_ERROR("failed to check the result of setlk with conflict lock"); + } + return 0; +} + +static int test_child_setlkw() { + struct flock fl = { F_RDLCK, SEEK_SET, 0, g_file_len / 4, 0 }; + int res = fcntl(g_fd, F_SETLKW, &fl); + if (res < 0 && errno != EINTR) { + THROW_ERROR("failed to check the result of setlkw with conflict lock"); + } + return 0; +} + +// ============================================================================ +// Test suite main +// ============================================================================ + +static test_case_t test_cases[] = { + TEST_CASE(test_getlk), + TEST_CASE(test_setlk), + TEST_CASE(test_spawn_child_and_unlock), +}; + +static test_case_t child_test_cases[] = { + TEST_CASE(test_child_getlk), + TEST_CASE(test_child_setlk), + TEST_CASE(test_child_setlkw), +}; + +int main(int argc, const char *argv[]) { + // Save argument for test cases + g_argc = argc; + g_argv = argv; + g_fd = open_or_create_file(); + if (g_fd < 0) { + THROW_ERROR("failed to open/create file"); + } + + // Test argc + if (argc == 2) { + if (test_suite_run(child_test_cases, ARRAY_SIZE(child_test_cases)) < 0) { + THROW_ERROR("failed run child test"); + } + // Donot close file intentionally to unlock the lock on exit + // close(g_fd); + } else { + if (test_suite_run(test_cases, ARRAY_SIZE(test_cases)) < 0) { + THROW_ERROR("failed run test"); + } + close(g_fd); + if (remove_file() < 0) { + THROW_ERROR("failed to remove file after test"); + } + } + return 0; +}