-
Notifications
You must be signed in to change notification settings - Fork 357
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
RDP file not being copied in some cases #3
Comments
Was looking at this one and have some thoughts on how to correct it, but not sure they are good ones as I cannot reproduce the bug. What I am thinking is happening is for some reason, windows is trying to be helpful (unsuccessfully) and is providing 2 different TEMP folders; one for RemoteApp Tool and one for RDP2MSI. So rdpParentFolder and TempPath are coming back with 2 different paths so rdpInTemp is false. This causes the file to be copied with overwrite while it is potentially in use by something (antivirus, windows indexer, end user leaving it open, etc). The other way I can see to fix this is to toss a TRY CATCH around the CopyFile calls on line 76 and 79 (as either one could fail) and in the event the copy fails, we catch it, change the rdpTempPath and/or icoTempPath to something unique (such as TempPath & "" & ProductBaseFileName & DateTime.Now.ToString("yyyyMMddHHmmss") &".rdp" or ".ico". The problem I have with making this change is I am not able to reproduce the problem, so I have no idea if my suggestions will fix it or not and therefore I am not confident about making a pull request for this fix. |
Do you know if this issue still exists? Part of me is wondering if it is an issue with Wix 3.10.3? |
Very good ideas! It's a shame we can't replicate the issue to test which one will work best. The reports on this have been inconsistent. I have not once been able to replicate the issue - I have built several Server 2016 VMs with no issues using the tool. My initial thought was that because Server 2016 includes Windows Defender real-time scanning, perhaps on slower systems, the temp file was still being scanned while RemoteApp Tool is trying to do something with it. People have emailed me reporting the problem but insisting they don't have any antivirus installed (but I suspect they might have been overlooking Windows Defender). Nobody has reported the problem for a while! Could be because it is resolved, or maybe just that people aren't using the tool on 2016. I didn't suspect WiX because it's all happening before WiX is called, but perhaps .NET/Windows queues the copy operation and it hasn't quite finished before the code continues. I'd be interested to use your TRY/CATCH idea to continually attempt the copy/delete operation until it succeeds (perhaps with a timeout/error). This would prevent future antivirus issues, even if that isn't the cause of the current problem. But also, happy to try any ideas, we just need to replicate the problem first! |
May have found a better solution to this: The code is C# specific, but I can convert that to VB.NET and should be able to implement it in this project. Should I work on implementing that? It looks pretty easy to do. |
I like that this would not only potentially fix the issue but also give users (and us) an insight into the problem! |
Probably not going to have time to implement this today, but should have something in place by Friday. |
Making progress on this. Don't have it working 100% yet, but it is coming along. Hopefully have something ready to test at some point tomorrow. UPDATE - Got it working to report back file locks, application that is causing the lock and the PID of the application as some things (like Excel) can have multiple PIDs. Did a quick test on it with an excel file and if I had it open, I got an error, if it was closed, no error. So now just need to implement this over to any place we call copy file , move file, create file or delete file. |
Pull request in place. Got the code working and tested. Feel free to go in and change things if you don't like the way I implemented it. This does NOT fix issue 3, just helps us debug what is going on. |
I appear to be experiencing this issue. Here is the debug output... See the end of this message for details on invoking ************** Exception Text ************** ************** Loaded Assemblies **************
|
To confirm, which antivirus are you running? |
Running Windows defender. Yes there is an rdp file in that location. Yes, I can delete it. |
Fixes Issue#3 * After some testing and fiddling with things, there is a case that can occur where the rdpTempPath and rdpFilePath can be the same. This results in the file lock check saying the file is NOT locked (as it is not at that point) BUT the application still throwing a JIT error * Correction is to check if the file path and temp path are the same and if so, rename the temp path. As it is temp, we can name it anything we want. I append a _2 to the end.
Fixes Issue#3 * After some testing and fiddling with things, there is a case that can occur where the rdpTempPath and rdpFilePath can be the same. This results in the file lock check saying the file is NOT locked (as it is not at that point) BUT the application still throwing a JIT error * Correction is to check if the file path and temp path are the same and if so, rename the temp path. As it is temp, we can name it anything we want. I append a _2 to the end.
Fixes Issue#3 * After some testing and fiddling with things, there is a case that can occur where the rdpTempPath and rdpFilePath can be the same. This results in the file lock check saying the file is NOT locked (as it is not at that point) BUT the application still throwing a JIT error * Correction is to check if the file path and temp path are the same and if so, rename the temp path. As it is temp, we can name it anything we want. I append a _2 to the end.
Did some work and have better handling now of the file locking which will hopefully fix the issue. Can I get someone to test out this version: |
Fixes Issue#3 * After some testing and fiddling with things, there is a case that can occur where the rdpTempPath and rdpFilePath can be the same. This results in the file lock check saying the file is NOT locked (as it is not at that point) BUT the application still throwing a JIT error * Correction is to check if the file path and temp path are the same and if so, rename the temp path. As it is temp, we can name it anything we want. I append a _2 to the end.
Fixes Issue#3 * After some testing and fiddling with things, there is a case that can occur where the rdpTempPath and rdpFilePath can be the same. This results in the file lock check saying the file is NOT locked (as it is not at that point) BUT the application still throwing a JIT error * Correction is to check if the file path and temp path are the same and if so, rename the temp path. As it is temp, we can name it anything we want. I append a _2 to the end.
Fixes Issue#3 * After some testing and fiddling with things, there is a case that can occur where the rdpTempPath and rdpFilePath can be the same. This results in the file lock check saying the file is NOT locked (as it is not at that point) BUT the application still throwing a JIT error * Correction is to check if the file path and temp path are the same and if so, rename the temp path. As it is temp, we can name it anything we want. I append a _2 to the end.
I am having the same issue as abcbarryn on Windows Server 2016 and 2019. I also tried your test version and the error seems to have moved to the Icon file instead of the rdp file. I have downloaded a copy of the code to look into the issue as well. I'll update if I find anything out. Here is a copy of the error. System.IO.IOException: The process cannot access the file 'C:\Users\astaggenborg\AppData\Local\Temp\ABAS2018.ico' because it is being used by another process. |
So the issue is not fixed in the test builds, it just moves off of the rdp file and jumps over to the ico. So that means (to me) progress! |
Now that I have everything setup on my dev machine, I ran from the code directly and it seems that the filelock code isn't catching the locked rdp file, and is crashing on the copyfile line. I will investigate it more tomorrow. |
I have not had a lot of time to look into this but my guess is one of 2 things: So the next revision I am going to work on is checking for locks on source and destination files and if locks are not in place it will do a try catch block over the copy. If the copy fails, it renames the destination file to have "_new" on the end. Then I can clean up that bit of code too and break it into a few more functions. The way I tested the lock check functionality was by using a file type I KNOW gets locks (word document) and tried to overwrite the word doc with the rdp file. I got a pop up telling me the file is locked. It is a fun one as I cannot reproduce it on any of my systems (windows server 2012, windows server 2019, windows 10), yet others seem to have no problem making it happen. I do appreciate another set of eyes on this as mine have gone buggy trying to figure this out. |
Fixes Issue kimmknight#3 This commit fixes issue kimmknight#3 by checking for the file lock on the source and destination files. If a lock is found, it asks the user what they want to do. After it is determined that no locks exist, it attempts a file copy. If the file copy fails, it renames the file to append _NEW onto the name and tries the copy a second time. If it fails the second time, it gives up but lets the user know to put in an issue on github and what information to provide. With my quick testing, it appears to work when the destination file is locked to rename it. The renamed version of the file is passed back to the calling function along with a success value. This should correct issue kimmknight#3 or at least give end users a chance to fix things on their own (delete the locked file manually). Also fixed a typo in the lock checking loop. Not entirely sure how it wasn't stuck in an infinite loop previously when checking for locks.
May have fixed this bug for good: Can I get someone out there to give that version a go? |
I just tried it out on my Server 2019 and it worked without issue. |
@MrBrianGale, has wk6768 possibly found the issue and fixed it with the change in pull request #79 ? Their original comment was: When the user name is "Administrator" |
Thanks for the howsome tool :) if this can be of any help ... logging to the Win10 ETS VM that host the app with an "adminxxxx" login name would lead to the issue. When the user name is "Administrator" |
I think you may have got the nail on the head (maybe). My guess is that adminabcd is getting truncated to admin~1 which is the administrator folder, not the adminabcd folder... |
https://groups.google.com/d/msg/remoteapptool/B6Ni3Xf_K68/o4ZlUHuhCAAJ
WinIO error when trying to copy/move the RDP file. Maybe antivirus? Is there a way to avoid this type of problem?
The text was updated successfully, but these errors were encountered: