Byron/google-apis-rs

mixed sync & async IO usage in GCS upload

danburkert opened this issue · 2 comments

The google-storage1 APIs use a mix of sync (std::io) and async APIs. For instance, the upload method takes an argument of ReadSeek which uses blocking IO, but the method itself is async. Looking through the implementation, it appears that ultimately the work is done in doit, which uses the synchronous ReadSeek instance without using something like tokio::task::block_in_place. My understanding of tokio is that such mixed sync and async usage is highly discouraged, since it will end up blocking unrelated async tasks while doing the sync IO operations. Am I missing something, or is this a real issue?

Byron commented

That's a great catch! When the library was converted to async, this trait was overlooked. Fixing it could certainly be done by switching it out with its async counterpart.

A PR with a fix would be greatly appreciated.

Note the same issue is present at google drive API - upload there uses sync IO for reading the files.

The simplest way to refactor this seems to kill the ReadSeek trait (which exposes the sync std::io::Read + std::io::Seek) and see what blows up.