From d81dcabe8edda69c379e99c6917bd75f44dff1e9 Mon Sep 17 00:00:00 2001 From: "Bruno G. Albuquerque" Date: Fri, 25 Apr 2008 19:45:40 +0000 Subject: [PATCH] FS gurus, please review. - get_new_fd() now checks if we are dealing with attributes before deciding to bail out on a locked vnode. - Enabled locking in MailSettings again as it now works. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@25162 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- src/kits/mail/MailSettings.cpp | 9 +++------ src/system/kernel/fs/vfs.cpp | 7 ++++++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/kits/mail/MailSettings.cpp b/src/kits/mail/MailSettings.cpp index 707763241c..0f5ca3aa50 100644 --- a/src/kits/mail/MailSettings.cpp +++ b/src/kits/mail/MailSettings.cpp @@ -55,15 +55,12 @@ NewMailChain() BDirectory chain_dir(path.Path()); BDirectory outbound_dir(&chain_dir,"outbound"), inbound_dir(&chain_dir,"inbound"); - // TODO(bga): We should lock the directory before reading its contents - // and updating the last_issued_chain_id to avoid race conditions in - // case 2 Chains are being created at the same time. Unfortunately, - // calling BNode::Lock() in the directory is preventing the attribute to - // written (and it should not). Add the locking back when this is fixed. - // TODO(bga): A better way to do all this anyway is to write the chain // information to the settings message (a new field would be added). + // Lock Chain directory to avoid concurrent access. + chain_dir.Lock(); + int32 id = -1; //-----When inc'ed, we start with 0---- chain_dir.ReadAttr("last_issued_chain_id",B_INT32_TYPE,0,&id,sizeof(id)); diff --git a/src/system/kernel/fs/vfs.cpp b/src/system/kernel/fs/vfs.cpp index a63c0be556..7dec4aa1c0 100644 --- a/src/system/kernel/fs/vfs.cpp +++ b/src/system/kernel/fs/vfs.cpp @@ -2524,7 +2524,12 @@ get_new_fd(int type, struct fs_mount *mount, struct vnode *vnode, int fd; // if the vnode is locked, we don't allow creating a new file descriptor for it - if (vnode && vnode->mandatory_locked_by != NULL) + // unless it is an attribute that is being created as, in this case, + // the caller is guaranteed to be the lock holder. + // TODO(bga): Is this really true? If I understand this correctly, only + // the lock owner would be able to reach the code here, but I am not + // 100% sure. + if (vnode && vnode->mandatory_locked_by != NULL && type != FDTYPE_ATTR) return B_BUSY; descriptor = alloc_fd();