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

Use of ECF IFileTransferPausable should allow for pause and resume to fail/return false #300

Open
scottslewis opened this issue Jul 9, 2023 · 1 comment

Comments

@scottslewis
Copy link

Currrently, the p2 FileReader class

https://github.com/eclipse-equinox/p2/blob/master/bundles/org.eclipse.equinox.p2.transport.ecf/src/org/eclipse/equinox/internal/p2/transport/ecf/FileReader.java

Uses the ECF IFileTransferPausable.pause() and resume() method to pause and resume file download.

The IFileTransferPausable interface is here

https://github.com/eclipse/ecf/blob/master/framework/bundles/org.eclipse.ecf.filetransfer/src/org/eclipse/ecf/filetransfer/IFileTransferPausable.java

Both the pause() and resume() methods return a boolean which indicates whether the pause/resume as successful or not.

Currently, the p2 ecf transport does not examine the return values from pause or resume and this can lead to p2 hanging waiting for an event that will never occur as in this issue:

#206 (comment)

This should be fixed so that FileReader fails gracefully if false is returned from either pause() or resume().

AFAICT, the p2 FileReader class is the only one that uses IFileTransferPausable.pause() or resume().

@laeubi
Copy link
Member

laeubi commented Jul 10, 2023

This should be fixed so that FileReader fails gracefully if false is returned from either pause() or resume().

I have take a look at FileReader usage of IFileTransferPausable.pause() at laest that looks safe for me:

  1. P2 first checks if it should pause the transfer and if it already has paused
  2. if not ECF is asked to pause, but no internal state is modified, so even if false is returned P2 will simply not pause
  3. That the transport has paused is only performed in the org.eclipse.equinox.internal.p2.transport.ecf.FileReader.handleTransferEvent(IFileTransferEvent) and thats the only location where it is set to true

So it seems that the pause() somehow make it to the ECF and P2 but the resume then fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants