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

FileInfoReader deadlock #568

Closed
jukzi opened this issue Nov 18, 2024 · 10 comments
Closed

FileInfoReader deadlock #568

jukzi opened this issue Nov 18, 2024 · 10 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@jukzi
Copy link
Contributor

jukzi commented Nov 18, 2024

          Maybe an infinite loop waiting for org.eclipse.equinox.internal.p2.transport.ecf.FileInfoReader.barrier

waitOnSelf() does not synchronize the read on that while org.eclipse.equinox.internal.p2.transport.ecf.FileInfoReader.handleRemoteFileEvent(IRemoteFileSystemEvent) does synchronize for write

Originally posted by @jukzi in eclipse-pde/eclipse.pde#1481 (comment)

original reports:
eclipse-pde/eclipse.pde#1481
eclipse-pde/eclipse.pde#1160

@laeubi
Copy link
Member

laeubi commented Nov 18, 2024

This code looks a bit suspicious on the other hand its there since 16 years now, so there are likely better ways today to solve this.

One part is also this one

it only notifies one thread, not all threads waiting... beside that it seem to be blocked in job.join() so has no chance to check the condition anyways...

jukzi pushed a commit to jukzi/p2 that referenced this issue Nov 18, 2024
The value of the barrier (true/false) was never read. It was only
checked for !=null. The value had been read in waitOnSelf() without any
multithreading semantics like synchronize/volatile.
Instead now we use a thread safe AtomicBoolean. The synchronization
blocks are still used for wait()/notifyAll()

eclipse-equinox#568
@jukzi
Copy link
Contributor Author

jukzi commented Dec 17, 2024

happens again - eclipse does not finish to start after update:
image
i sampled a bit:
image

@jukzi jukzi added the help wanted Extra attention is needed label Dec 17, 2024
@laeubi
Copy link
Member

laeubi commented Dec 17, 2024

Thread State / Dump would be more interesting than CPU sampling. e.g. why does PDE hinders the IDE to start at all?

@jukzi
Copy link
Contributor Author

jukzi commented Dec 17, 2024

Thread State / Dump would be more interesting than CPU sampling. e.g. why does PDE hinders the IDE to start at all?

see eclipse-pde/eclipse.pde#1481 (comment)

@laeubi
Copy link
Member

laeubi commented Dec 17, 2024

From that stacktrace it seem to hang in a Java Editor that triggers init of classpath while showing a view (what triggers PDE to do something).

Still I'm wondering why all this happens before the UI is displayed....

@laeubi
Copy link
Member

laeubi commented Jan 7, 2025

I have now analyzed this in more details, and the problem is IDEApplication.start(IApplicationContext) does suspend the JobManager, so no jobs are ever scheduled. Therefore the condition will never be meet and the code is running endlessly. It seems that this is particular happen with targlets (I can't reproduce this with plain PDE).

I tired to do some fixing for this in the FileInfoReader itself, but the problem is that ECF uses a job internally as well (that then will never run of course) so it does not help here much.

@laeubi
Copy link
Member

laeubi commented Jan 9, 2025

The class has been overhauled now to not using any jobs or simple synchronization primitives so I think its safe to close this issue.

@laeubi laeubi closed this as completed Jan 9, 2025
@vogella
Copy link
Contributor

vogella commented Jan 9, 2025

Thanks for working on this @laeubi

@jukzi
Copy link
Contributor Author

jukzi commented Jan 10, 2025

i think it should be reopened since org.eclipse.ecf.provider.filetransfer.browse.AbstractFileSystemBrowser.sendBrowseRequest() still uses Job.schedule whiich will not work with a suspended JobManager!

@laeubi
Copy link
Member

laeubi commented Jan 10, 2025

see eclipse/ecf#154 so its nothing P2 can change here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants