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

No prefetch in "stream_open()" [SFTP] #40459

Closed
wants to merge 1 commit into from

Conversation

z49x2vmq
Copy link

@z49x2vmq z49x2vmq commented Sep 16, 2023

Summary

$this->request_chunk(256 * 1024) at the end of stream_open() fetches a chunk of file contents. But when stream_close() is called without calling stream_read(), it tries to read status code from request_chunk(). Which lead to stream_close() to return false.

After this, regular files are displayed as folders. Previews are not generated.

Instead of fetching chunk in stream_open(), make the stream_read() call both request_chunk() and read_chunk() it it.

TODO

  • Discuss

Checklist

"$this->request_chunk(256 * 1024);" at the end of "stream_open()"
fetches a chunk of file contents. But when "stream_close()" is called
without calling "stream_read()", it tries to read status code from
"request_chunk()". Which lead to stream_close() to return false.

After this, regular files are displayed as folders. Previews are not
generated.

Instead of fetching chunk in "stream_open()", make the "stream_read()"
call both "request_chunk()" and "read_chunk()" it it.

Issue nextcloud#25788

Signed-off-by: Rae Kim <rae.kim@raekim.me>
@solracsf solracsf added the 3. to review Waiting for reviews label Sep 16, 2023
@solracsf solracsf added this to the Nextcloud 28 milestone Sep 16, 2023
@szaimen szaimen requested review from come-nc, a team, icewind1991 and nfebe and removed request for a team September 16, 2023 18:22
@joshtrichards joshtrichards changed the title No prefetch in "stream_open()" No prefetch in "stream_open()" [SFTP] Sep 17, 2023
@joshtrichards
Copy link
Member

/update-3rdparty/

@icewind1991
Copy link
Member

#40183 fixes the same "close error" while maintaining the pre-fetch performance benefits.

@z49x2vmq
Copy link
Author

@icewind1991 Great. Let me close this one. Thank you for the fix

@z49x2vmq z49x2vmq closed this Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SFTP External Storage error
4 participants