Skip to content

Commit 056ffe3

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 91ad32f commit 056ffe3

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 - 2020 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
@@ -120,14 +120,15 @@ public stateid4 addOpen(NFS4Client client, StateOwner owner, Inode inode, int sh
120120
throw new ShareDeniedException("Conflicting share");
121121
}
122122

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

126126
for (OpenState os : opens) {
127127
if (os.client.getId() == client.getId() &&
128128
os.getOwner().equals(owner)) {
129129
os.shareAccess |= shareAccess;
130130
os.shareDeny |= shareDeny;
131+
os.stateid.seqid++;
131132
return os.stateid;
132133
}
133134
}
@@ -137,6 +138,7 @@ public stateid4 addOpen(NFS4Client client, StateOwner owner, Inode inode, int sh
137138
OpenState openState = new OpenState(client, owner, stateid, shareAccess, shareDeny);
138139
opens.add(openState);
139140
state.addDisposeListener(s -> removeOpen(inode, stateid));
141+
stateid.seqid++;
140142
return stateid;
141143
} finally {
142144
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)