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

Start-GSDriveFileUpload does not release file lock when upload is complete #144

Closed
richmcd opened this issue Jan 14, 2019 · 17 comments
Closed
Assignees

Comments

@richmcd
Copy link

richmcd commented Jan 14, 2019

When uploading a file from the local system to Google Drive using the Start-GSDriveFileUpload cmdlet the file completes but is not able to be modified, moved, or deleted once the upload is complete until the PowerShell session is closed. This seems to be because the upload does not clean itself up or release the file lock when it completes. This makes scripting of uploads that then archive or clean up the original files difficult. Either the upload needs to clean itself up after completing successfully, or at least release the lock on the uploaded file.

This could also be accomplished with a new cmdlet to Stop-GSDriveFileUploads, preferably one that takes objects from Get-GSDriveFileUplaodStatus on the pipeline, or add a switch or parameter to Get-GSDriveFileUploadStatus which would clean up the completed upload(s).

My specific case involved uploading a password protected XLSX file to a folder on a team drive using Start-GSDriveFileUpload with the -Wait switch. My expected behavior was to have no lingering upload tasks, or at least have some method to clear them once verified without having to close the session.

@scrthq
Copy link
Member

scrthq commented Jan 14, 2019

@richmcd - Thanks for opening this up! The $stream.Dispose() call is set as the ContinueWith task on the async upload request. I noticed that it doesn't immediately drop off either, but eventually does. Filestream is ideal for large files in comparison to MemoryStream, but one option could be to read in the bytes of the file into a memory stream instead of reading directly from the file and therefore locking it.

Let me do some playing around with it and see if I can add in the additional call to run the continuation task and dispose of the stream ASAP once the upload is complete! Adding Stop-GSDriveFileUpload could be useful too!

@scrthq
Copy link
Member

scrthq commented Jan 14, 2019

Did a bit more testing and it looks like adding ([System.IO.FileShare]::Delete + [System.IO.FileShare]::ReadWrite) to the parameter values passed to the following line allows read/write/delete access while the stream is still open:

https://github.com/scrthq/PSGSuite/blob/master/PSGSuite/Public/Drive/Start-GSDriveFileUpload.ps1#L253

Working on updating that tonight! I'll update here once this has been deployed to the PS Gallery!

@scrthq
Copy link
Member

scrthq commented Jan 14, 2019

One thing to note though is that, until the stream is disposed, the removed file will be visible in the filesystem but access will be locked. The code around UploadAsync for Start-GSDriveFileUpload does have a continuation task to Dispose() the stream, it just seems to take a couple minutes after the upload completes to actually Dispose()

To visualize this, you can use the following code (just update the first line with a test file):

$file = (Resolve-Path .\README.md).Path
$stream = New-Object 'System.IO.FileStream' $file,([System.IO.FileMode]::Open),([System.IO.FileAccess]::Read),([System.IO.FileShare]::Delete + [System.IO.FileShare]::ReadWrite)
Remove-Item $file -Force
Get-Content $file | Out-Null #1
$stream.Dispose()
Get-Content $file | Out-Null #2

You should get an Access Denied error as well as a Cannot find path.... error during the #1 Get-Content line. If you stop here, you should still be able to see the file in the file system (but it will be inaccessible).

As soon as $stream.Dispose() runs, you should see the file disappear from the filesystem as well, with #2 Get-Content only returning the Cannot find path.... error as expected:

image

@richmcd
Copy link
Author

richmcd commented Jan 14, 2019

Thanks @scrthq, it's odd that $stream.Dispose() should take so long in the normal circumstance. Might have something to do with the upload session object still having a reference to it or something garbage collection related.

@scrthq
Copy link
Member

scrthq commented Jan 14, 2019

@richmcd - yeah, it looks like something to do with UploadAsync() not running the continuation task as soon as it's complete. It could also be purely due to unfamiliarity with how those interact from my end. Running $stream.Dispose() definitely has the desired effect immediately, so something is delaying that stream disposal longer than expected. It definitely seems like it's happening eventually, as the files do unlock without closing the session after about a minute or so, just not sure how to kickstart that to happen ASAP or even have another call to run the Dispose() as soon as it sees it's complete.

Going to work on a couple options including adding in the $stream object to the upload list so its Dispose() method is available without reliance on the ContinuationTask.

@scrthq
Copy link
Member

scrthq commented Jan 15, 2019

@richmcd - PSGSuite v2.22.2 is now deployed! Here's the quick summary of updates and how they relate to your workflow:

  • Updated: Start-GSDriveFileUpload to Dispose() open streams once uploads are completed.
    • This should expedite disposal of open streams as soon as they are complete without any additional work required.
  • Added: Stop-GSDriveFileUpload to enable cleanup of any remaining open streams.
    • This will allow you to run Stop-GSDriveFileUpload after pressing Ctrl+C to cancel the Start-GSDriveFileUpload call and dispose any remaining open streams. This must be done from the same session you were uploading from due to queue scope.
  • Updated: Get-GSDriveFileUpload to Dispose() any completed streams that are still open.
    • If you did not run Start-GSDriveFileUpload with the -Wait switch, manually running Get-GSDriveFileUpload will perform the same cleanup tasks for any completed and non-disposed streams.

@richmcd
Copy link
Author

richmcd commented Jan 15, 2019

Sounds awesome @scrthq! has this been pushed to the gallery?

@scrthq
Copy link
Member

scrthq commented Jan 15, 2019

@richmcd yup!

image

@richmcd
Copy link
Author

richmcd commented Jan 15, 2019

Sweet! Thanks for your work on this. This is one of the most useful modules I've ever seen and appreciate your responsiveness here.

@scrthq
Copy link
Member

scrthq commented Jan 15, 2019

@richmcd my pleasure!! I'm glad it's working well for you!! Let me know how the changes work for ya and if you have any issues with the new version!!

@richmcd
Copy link
Author

richmcd commented Jan 15, 2019

@scrthq, The new version solves the issue, but seems to have introduced a new one.
image

Edit: This doesn't affect the actual upload and I can ignore those errors, but it might trip up some scripts.

@scrthq
Copy link
Member

scrthq commented Jan 15, 2019

@richmcd - Interesting! Are you able to run the following and if so, does it throw an error or just work?

[System.Console]::CursorVisible
[System.Console]::CursorVisible = $false
[System.Console]::CursorVisible = $true

This is in place to re-show the cursor when ran in a non-Windows PowerShell console (i.e. VS Code or *nix terminal), but I can remove it if it's causing more harm than good. I can't reproduce the error on my end though.

Also, what version of PS are you using?

@scrthq scrthq reopened this Jan 15, 2019
@richmcd
Copy link
Author

richmcd commented Jan 15, 2019

@scrthq, I'm using Windows PowerShell 5.1, but interesting note, it only seems to happen in the PowerShell ISE console. I can run those commands in a standard PowerShell console without issue.

@scrthq
Copy link
Member

scrthq commented Jan 15, 2019 via email

@scrthq
Copy link
Member

scrthq commented Jan 15, 2019

@richmcd v2.22.3 pushed with the additional guard around that call to [Console]::CursorVisible when using ISE. You should be good to go!

@scrthq scrthq reopened this Jan 15, 2019
@richmcd
Copy link
Author

richmcd commented Jan 15, 2019

@scrthq, Yup issue fixed. Thanks again.

@scrthq
Copy link
Member

scrthq commented Jan 16, 2019

Perfect! You bet!!

@scrthq scrthq closed this as completed Jan 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants