From abc0aa3bd2a68a9ef3650df6315459c4a65caaf9 Mon Sep 17 00:00:00 2001 From: Tigran Mkrtchyan Date: Wed, 29 Mar 2023 19:42:01 +0200 Subject: [PATCH] nfs4: FileTracker should return a copy of stateid Motivation: As parallel threads modifies open-stateid sequence number the FileTracker should always return a copy to avoid that sequence is modified before reply is sent. Modification: Update FileTracker#addOpen and FileTracker#downgradeOpen to return a copy of stateid. Result: Spec compliant behaviour in concurrent environment. Fixes: #96 Acked-by: Albert Rossi Target: master, 0.24, 0.23 (cherry picked from commit 73d73afc094bd126611358ec2f68e2b75a3147a9) Signed-off-by: Tigran Mkrtchyan --- .../main/java/org/dcache/nfs/v4/FileTracker.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/dcache/nfs/v4/FileTracker.java b/core/src/main/java/org/dcache/nfs/v4/FileTracker.java index 3393d0ad..6cb1d234 100644 --- a/core/src/main/java/org/dcache/nfs/v4/FileTracker.java +++ b/core/src/main/java/org/dcache/nfs/v4/FileTracker.java @@ -95,7 +95,7 @@ public StateOwner getOwner() { * @param inode of opened file. * @param shareAccess type of access required. * @param shareDeny type of access to deny others. - * @return stateid associated with open. + * @return a snapshot of the stateid associated with open. * @throws ShareDeniedException if share reservation conflicts with an existing open. * @throws ChimeraNFSException */ @@ -129,7 +129,8 @@ public stateid4 addOpen(NFS4Client client, StateOwner owner, Inode inode, int sh os.shareAccess |= shareAccess; os.shareDeny |= shareDeny; os.stateid.seqid++; - return os.stateid; + //we need to return copy to avoid modification by concurrent opens + return new stateid4(os.stateid.other, os.stateid.seqid); } } @@ -139,7 +140,8 @@ public stateid4 addOpen(NFS4Client client, StateOwner owner, Inode inode, int sh opens.add(openState); state.addDisposeListener(s -> removeOpen(inode, stateid)); stateid.seqid++; - return stateid; + //we need to return copy to avoid modification by concurrent opens + return new stateid4(stateid.other, stateid.seqid); } finally { lock.unlock(); } @@ -153,7 +155,7 @@ public stateid4 addOpen(NFS4Client client, StateOwner owner, Inode inode, int sh * @param inode of opened file. * @param shareAccess type of access required. * @param shareDeny type of access to deny others. - * @return stateid associated with open. + * @return a snapshot of the stateid associated with open. * @throws ChimeraNFSException */ public stateid4 downgradeOpen(NFS4Client client, stateid4 stateid, Inode inode, int shareAccess, int shareDeny) throws ChimeraNFSException { @@ -182,7 +184,8 @@ public stateid4 downgradeOpen(NFS4Client client, stateid4 stateid, Inode inode, os.shareDeny = shareDeny; os.stateid.seqid++; - return os.stateid; + //we need to return copy to avoid modification by concurrent opens + return new stateid4(os.stateid.other, os.stateid.seqid); } finally { lock.unlock(); }