Skip to content

Commit 6e2658a

Browse files
committed
nfs4: bump open-stateid sequence only in FileTracker#addOpen
Motivation: To avoid race conditions, the updating of the open-stateid must happen only under lock, which is guaranteed by FileTracker#addOpen Modification: Update open-stateid sequence only in FileTracker#addOpen Result: Fix of the race condition Fixes: #120 Acked-by: Paul Millar Target: master, 0.24, 0.23 (cherry picked from commit 42ed2e0) Signed-off-by: Tigran Mkrtchyan <[email protected]>
1 parent 98033b3 commit 6e2658a

File tree

3 files changed

+7
-5
lines changed

3 files changed

+7
-5
lines changed

core/src/main/java/org/dcache/nfs/v4/FileTracker.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017 - 2022 Deutsches Elektronen-Synchroton,
2+
* Copyright (c) 2017 - 2023 Deutsches Elektronen-Synchroton,
33
* Member of the Helmholtz Association, (DESY), HAMBURG, GERMANY
44
*
55
* This library is free software; you can redistribute it and/or modify
@@ -125,14 +125,15 @@ public stateid4 addOpen(NFS4Client client, StateOwner owner, Inode inode, int sh
125125
throw new ShareDeniedException("Conflicting share");
126126
}
127127

128-
// if there is an another open from the same client we must merge
128+
// if there is another open from the same client we must merge
129129
// access mode and return the same stateid as required by rfc5661#18.16.3
130130

131131
for (OpenState os : opens) {
132132
if (os.client.getId() == client.getId() &&
133133
os.getOwner().equals(owner)) {
134134
os.shareAccess |= shareAccess;
135135
os.shareDeny |= shareDeny;
136+
os.stateid.seqid++;
136137
return os.stateid;
137138
}
138139
}
@@ -142,6 +143,7 @@ public stateid4 addOpen(NFS4Client client, StateOwner owner, Inode inode, int sh
142143
OpenState openState = new OpenState(client, owner, stateid, shareAccess, shareDeny);
143144
opens.add(openState);
144145
state.addDisposeListener(s -> removeOpen(inode, stateid));
146+
stateid.seqid++;
145147
return stateid;
146148
} finally {
147149
lock.unlock();

core/src/main/java/org/dcache/nfs/v4/OperationLOCK.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2009 - 2018 Deutsches Elektronen-Synchroton,
2+
* Copyright (c) 2009 - 2023 Deutsches Elektronen-Synchroton,
33
* Member of the Helmholtz Association, (DESY), HAMBURG, GERMANY
44
*
55
* This library is free software; you can redistribute it and/or modify
@@ -122,6 +122,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF
122122
context.getLm().unlockIfExists(inode.getFileId(), lock);
123123
});
124124

125+
// FIXME: we might run into race condition, thus updating sedid must be fenced!
125126
lock_state.bumpSeqid();
126127
context.currentStateid(lock_state.stateid());
127128
result.oplock.status = nfsstat.NFS_OK;

core/src/main/java/org/dcache/nfs/v4/OperationOPEN.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2009 - 2020 Deutsches Elektronen-Synchroton,
2+
* Copyright (c) 2009 - 2023 Deutsches Elektronen-Synchroton,
33
* Member of the Helmholtz Association, (DESY), HAMBURG, GERMANY
44
*
55
* This library is free software; you can redistribute it and/or modify
@@ -271,7 +271,6 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF
271271
_args.opopen.share_access.value,
272272
_args.opopen.share_deny.value);
273273

274-
stateid.seqid++;
275274
context.currentStateid(stateid);
276275
res.resok4.stateid = stateid;
277276
res.status = nfsstat.NFS_OK;

0 commit comments

Comments
 (0)