Bugzilla – Full Text Bug Listing
| Summary: | Failure reading zipped repository is causing PDE/Build test failure | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Andrew Niefer <aniefer> | ||||
| Component: | p2 | Assignee: | Meng Xin Zhu <kane.zhu> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | irbull, john.arthorne, kane.mx, kane.zhu, pascal, slewis, tjwatson | ||||
| Version: | 3.8.0 Juno | ||||||
| Target Milestone: | Juno M6 | ||||||
| Hardware: | PC | ||||||
| OS: | Linux | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 368246 | ||||||
| Attachments: |
|
||||||
|
Description
Andrew Niefer
This particular stack trace has not happened in a recent IBuild. Also, both DJ and I have run the p2 tests locally and not seen this either. However, there are a few tests that are failing related to unpacking. These happened recently on the 1.7 VM test machine. Could be related to that. I'll keep an eye out. I can reproduce this. I traced it as far as: org.eclipse.ecf.internal.provider.filetransfer.Activator#getFileTransfer(String protocol) When asked for the protocol handler for the "jar" protocol, it comes up empty. We received a new ECF in the past month or so, is it possible they removed support for the "jar" protocol? Hmmm....No, we/ECF haven't done anything to remove support for the jar protocol. The only thing in the most recent version was the addition of support for reusing connections in the httpclient provider. Could someone post the jar that seems to be at issue...and I'll give it a try in ECF test code to see if I can reproduce. Thanks. Created attachment 210725 [details]
File being read
Here is the URI passed to the ECF file transfer API in my case:
jar:file:/C:/1target/junit-workspace/pde.build/publishShape/I.TestBuild/f-TestBuild-group.group.group.zip!/
I might have missed it, but I can't find an ECF extension for the retrieveFileTransferProtocolFactory extension point for the "jar" protocol in any of the ECF bundles we ship. (In reply to comment #5) > I might have missed it, but I can't find an ECF extension for the > retrieveFileTransferProtocolFactory extension point for the "jar" protocol in > any of the ECF bundles we ship. I believe it's handled by the jre URL protocol handler...as with file. I created a simpler p2 test case with identical jar and it worked. So the problem is not so simple and doesn't seem related to ECF. Thanks for your quick comment Scott. This bug was introduced by the fix for bug 364929. In that bug Meng introduced a static field on ProgressStatistics for the provisioning agent. The problem is, the agent cannot be static because there can be multiple agents running at once. In particular, p2 directory application creates its own agent, which is used in PDE build scenarios. Here is what is happening: - PDE build runs, invoking p2 directory - ProgressStatistics.agent gets set to the director's agent - p2 director completes, and stops its agent - We then try to perform a download, and it fails because the agent has been stopped. The IllegalStateException gets swallowed and it just appears to be a transfer failure. For reference here is the stack trace of the underlying failure: ProvisioningAgent.checkRunning() line: 85 ProvisioningAgent.getService(String) line: 49 ProgressStatistics.publishEvent(DownloadProgressEvent) line: 58 ProgressStatistics.report() line: 147 FileReader.handleTransferEvent(IFileTransferEvent) line: 164 UrlConnectionRetrieveFileTransfer(AbstractRetrieveFileTransfer).fireReceiveStartEvent() line: 671 UrlConnectionRetrieveFileTransfer.openStreams() line: 330 UrlConnectionRetrieveFileTransfer(AbstractRetrieveFileTransfer).sendRetrieveRequest(IFileID, IFileRangeSpecification, IFileTransferListener, Map) line: 889 UrlConnectionRetrieveFileTransfer(AbstractRetrieveFileTransfer).sendRetrieveRequest(IFileID, IFileTransferListener, Map) line: 576 MultiProtocolRetrieveAdapter.sendRetrieveRequest(IFileID, IFileTransferListener, Map) line: 106 FileReader.sendRetrieveRequest(URI, OutputStream, FileReader$DownloadRange, boolean, IProgressMonitor) line: 349 FileReader.readInto(URI, OutputStream, long, IProgressMonitor) line: 295 RepositoryTransport.download(URI, OutputStream, long, IProgressMonitor) line: 87 RepositoryTransport.download(URI, OutputStream, IProgressMonitor) line: 137 CacheManager.updateCache(File, URI, long, SubMonitor) line: 333 CacheManager.createCache(URI, String, IProgressMonitor) line: 205 SimpleMetadataRepositoryFactory.getLocalFile(URI, IProgressMonitor) line: 66 SimpleMetadataRepositoryFactory.load(URI, int, IProgressMonitor) line: 88 MetadataRepositoryManager.factoryLoad(URI, IExtension, int, SubMonitor) line: 57 MetadataRepositoryManager(AbstractRepositoryManager).loadRepository(URI, String, String, int, SubMonitor) line: 749 MetadataRepositoryManager(AbstractRepositoryManager).loadRepository(URI, IProgressMonitor, String, int) line: 651 MetadataRepositoryManager.loadRepository(URI, int, IProgressMonitor) line: 96 MetadataRepositoryManager.loadRepository(URI, IProgressMonitor) line: 92 PublishingTests(P2TestCase).loadMetadataRepository(URI) line: 99 Meng, can you take a look at fixing your work on ProgressStatistics? This class cannot have a static agent. I see you are setting it in some places before calling a repository, but there could be multiple threads in play. A couple of options: - Agent gets passed in to the Transport and set as an instance field on ProgressStatistics - Only use the agent of the currently running system - this means you won't get progress statistics for multi-agent scenarios such as the p2 director application. I have released a p2 test case that illustrates the problem: http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=5d2d6401627c5851207411a25c44dd56c17958da See JarURLMetadataRepositoryTest._testZipFileRepository At this point I propose that we revert the commit from bug #364929. Aside from the problem it causes now, I disagree with the approach taken to pass in this parameter and litters the API with very specific fields. We should discuss a better / more generic approach (my bad for not doing it earlier). John, I'll take it. The change of ProgressStatistics is NOT for bug 364929, it's made for bug 316328. The download progress was reported by download thread to update IProgressMonitor with predefined format. It's hard to be read by users if there are multiple downloading threads. The text is quickly updated by each thread, no overall download speed and estimated time. The change of ProgressStatistics is intend to broadcast the downloading progress via ProvisioningEventBus. It's originally designed as utility class, I'll try to make it suit multiple agents. (In reply to comment #8) > This bug was introduced by the fix for bug 364929. In that bug Meng introduced a > static field on ProgressStatistics for the provisioning agent. The problem is, > the agent cannot be static because there can be multiple agents running at once. > In particular, p2 directory application creates its own agent, which is used in > PDE build scenarios. Here is what is happening: > > - PDE build runs, invoking p2 directory > - ProgressStatistics.agent gets set to the director's agent > - p2 director completes, and stops its agent > - We then try to perform a download, and it fails because the agent has been > stopped. The IllegalStateException gets swallowed and it just appears to be a > transfer failure. > I submitted a fix[1]. Pls help review it. Thanks. Each agent has its Transport service, so the implementation of Transport could have the instance of agent which creates it. And somewhere are using RepositoryTransport as an utility class to download content, hence the agent is not mandatory for constructing an instance of RepsotiroyTransport. Regarding to P2 provisioning operations they get Transport service from agent, it means that the agent always is available for Transport service used by provisioning. Download progress events can be broadcasted during the provisioning. [1] http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=91027ecca979ba174a9617241ff3999250490ae9 (In reply to comment #13) > I submitted a fix[1]. Pls help review it. Thanks. > > Each agent has its Transport service, so the implementation of Transport could > have the instance of agent which creates it. And somewhere are using > RepositoryTransport as an utility class to download content, hence the agent is > not mandatory for constructing an instance of RepsotiroyTransport. Regarding to > P2 provisioning operations they get Transport service from agent, it means that > the agent always is available for Transport service used by provisioning. > Download progress events can be broadcasted during the provisioning. > > [1] > http://git.eclipse.org/c/equinox/rt.equinox.p2.git/commit/?id=91027ecca979ba174a9617241ff3999250490ae9 This fix looks good to me. It removes all statics and ensures the progress statistics class is publishing to the appropriate agent for that instance. Thanks John for all the help on this bug, and Meng, thanks for getting a patch together so quickly. The patch looks good in that it removes the uses of ServiceHelper to get static agent instances. I'll merge this to our integration branch. Pascal, if you want to review / discuss bug #364929, let's take that onto another bug report. Close it. |