Byron / google-apis-rs

A binding and CLI generator for all Google APIs
http://byron.github.io/google-apis-rs
Other
1.01k stars 131 forks source link

mixed sync & async IO usage in GCS upload #415

Open danburkert opened 1 year ago

danburkert commented 1 year ago

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 1 year ago

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.

MOZGIII commented 1 year ago

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.