Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manage NtStatus.NT_STATUS_OBJECT_NAME_COLLISION in SmbFile.mkDir #338

Open
gomezdegomera opened this issue Jul 6, 2023 · 7 comments
Open

Comments

@gomezdegomera
Copy link

Manage NtStatus.NT_STATUS_OBJECT_NAME_COLLISION error code on SmbFile.mkDir raises a SmbException in case of high concurrency level.
I noticed that for not known reasons this error code does not raise an exception on similar method mkDirs (note the final "s").
This has been justified by the following comment

// Ignore "Cannot create a file when that file already exists." errors for now as
// they seem to be show up under some conditions most likely due to timing issues.
if ( e.getNtStatus() != NtStatus.NT_STATUS_OBJECT_NAME_COLLISION ) {
                   throw e;
 }

Is it possibile to extend to mkDir method the same cautional behaviour?

@mbechler
Copy link
Contributor

mbechler commented Jul 9, 2023

I'd rather not generally suppress that error and also that might be an unexpected change for others. Can't you just use mkdirs then or catch the exception?

@gomezdegomera
Copy link
Author

Thanks for response.

The fact is that I'm using vfs and the problem arises randomly when I execute fileObject.getContent().getOutputStream() method, when some folders in the path given in input need to be created by getOutputStream (using mkdir method, not mkdirs, unfortunatly). This is the code I use.

var path = "smb://<server>/dir1/dir2/dir2/file1.txt";
            FileSystemManager fs = VFS.getManager();
            FileObject fileObject = fs.resolveFile(path, new FileSystemOptions());
            //createParentFileObjects(fileObject, dstPath);
            try (BufferedOutputStream out1 = new BufferedOutputStream(fileObject.getContent().getOutputStream())) {
                IOUtils.copy(inputStream, out1);    
            }

There are two processes using the code above, running concurrently that write different files but possibly on same sub paths.

For now I added createParentFileObjects method to mitigate the exception.

protected void createParentFileObjects(FileObject fileObject, String dstPath) throws Exception {
        for (int i = 1; i <= createParentFileObjectsAttempts; i++) {
            if ( i > 1 ) {
                ThreadUtils.safeSleep(1000);
            }
            try {
                if (fileObject.getParent() != null && !fileObject.getParent().exists()) {
                    fileObject.getParent().createFolder();
                }
                break;
            } catch (FileSystemException e) {
                if (e.getCause() instanceof SmbException && ((SmbException) e.getCause()).getNtStatus() == NtStatus.NT_STATUS_OBJECT_NAME_COLLISION) {
                    log.warn(String.format("smb object name collision when creating parent object of file object %s", dstPath, e));
                } else {
                    throw e;
                }
            }
        }
    }

Th problem is that also with createParentFileObjectsAttempts = 10, the collision exception randomly arises from fileObject.getContent().getOutputStream(), executed after all those attempts.

This is very strange, because for what I can understand, mkdir checks if a folder exists (and so do I in createParentFileObjects) before creating it, and even if there could be timing issues due to the fact that checking and creating are not synchronized on the two different processes, all these reiterative attempts should succeed.

Consider also that the path is long 4 folders max, also, and I noticed that, when the exception arises at last from getOutputStream only on of the two processes made those attempts, the other does not log any warning.

So, I realized, that hiding in mkdir that exception, could not be a solution, but what do you suggest to avoid the collision issue? What did you understand, about this issue, that brought you to hide that exception in mkdirs?

Thanks

@mbechler
Copy link
Contributor

Back then, I concluded that the issue was caused by a race between multiple existance checks and the file creation, but that may not have been the whole story.
However, Attributes/existance are cached per SmbFile instance (by default for 5 seconds), so this still could be the issue in your example.
You could try to adjust the jcifs.smb.client.attrExpirationPeriod system property, setting it to zero might work. The function to clear the cache explicitly (clearAttributeCache) unfortunately is currently not exposed.
If the issue still persists then, we might be looking at a more subtle issue.

@gomezdegomera
Copy link
Author

Thanks, it seems to work. I will do some stress test to be sure!

@gomezdegomera
Copy link
Author

Unfortunatly, I've spoken too early, randomly the exception occurs even setting that prop to 0.

jcifsProperties.setProperty("jcifs.smb.client.enableSMB2", "true"); jcifsProperties.setProperty("jcifs.smb.client.useSMB2Negotiation", "true");
jcifsProperties.setProperty("jcifs.smb.client.attrExpirationPeriod", "0");
CIFSContext jcifsContext = new BaseContext(new PropertyConfiguration(jcifsProperties)); 

Are there other props I can set differently to figure it out?

Thanks

@mbechler
Copy link
Contributor

So you are still encountering repeated error for the same parent?
The next logical step would be to look at a debug log and/or packet capture to establish what is sent by which party at what point.

@gomezdegomera
Copy link
Author

Unfortunatly I could not do that yet, so I have no other information to share about this problem. I will update the issue with other info when possibile, but if you discover something about this problem in the meanwhile, update this issue, thanks. I will try other libraries to check if the problem exists in general, however

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants