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

Recover from incorrect JDK #441

Merged
merged 5 commits into from
Dec 10, 2019
Merged

Recover from incorrect JDK #441

merged 5 commits into from
Dec 10, 2019

Conversation

mzarnowski
Copy link
Contributor

@mzarnowski mzarnowski commented Nov 22, 2019

This adds a "Refresh Pants Project SDK" action to Pants drop-down menu.
When activated, this action will refresh the sdk of the current project.

If the SDK is not set or is not a pants-generated one, a pants SDK will be generated if needed and set as a project's SDK. If it already is a pants-generated SDK, it will be regenerated.

Copy link
Collaborator

@wisechengyi wisechengyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Marek!

The issue seems a bit trickier than it appears, because there are two symlink redirections in our configuration, but it doesn't make much sense for the plugin to detect the chain of symlinks IMO, e.g.
1.

# ls -l /Library/Java/JavaVirtualMachines/TwitterJDK
lrwxr-xr-x  1 root  wheel  40 Nov 23 13:28 /Library/Java/JavaVirtualMachines/TwitterJDK -> /opt/twitter_mde/package/TwitterJDK/current
# ls -l /opt/twitter_mde/package/TwitterJDK/current
lrwxr-xr-x  1 root  managed_dev_env  3 Nov 23 13:14 /opt/twitter_mde/package/TwitterJDK/current -> xyz

So, I was thinking instead of detecting and changing the SDK automatically, how about we make it an action called Refresh Pants Project SDK under the Pants menu, leaving it to user to decide when to trigger the refresh the SDK configuration?

Screen Shot 2019-11-23 at 1 36 19 PM

The status quo was

  1. User has to remove the existing SDK
  2. Reimport the project which then recreates the SDK

With the improvement, user only needs to trigger the Refresh Pants Project SDK while the project is open.

@mzarnowski
Copy link
Contributor Author

how about we make it an action called Refresh Pants Project SDK

That sounds way simpler, so than relying on some automagic, so I agree it might be preferable.
I have updated the PR with said action and removed previous logic for both automatically detecting changes in the JDK directory and refreshing stale JDK on project startup.

@wisechengyi
Copy link
Collaborator

Looks like this the refresh action is running on the UI thread, causing:

java.lang.Throwable: Synchronous execution on EDT: /Users/yic/workspace/pants/pants --no-quiet export --no-colors --export-output-file=/var/folders/yt/pxdd3gp96530xh_116kskmkh0000gp/T/pants_export_run7079551142178118023.out
	at com.intellij.openapi.diagnostic.Logger.error(Logger.java:145)
	at com.intellij.execution.process.OSProcessHandler.checkEdtAndReadAction(OSProcessHandler.java:147)
	at com.intellij.execution.process.OSProcessHandler.waitFor(OSProcessHandler.java:85)
	at com.intellij.execution.process.CapturingProcessRunner.runProcess(CapturingProcessRunner.java:29)
	at com.intellij.execution.process.CapturingProcessHandler.runProcess(CapturingProcessHandler.java:55)
	at com.twitter.intellij.pants.util.PantsUtil.getCmdOutput(PantsUtil.java:685)
	at com.twitter.intellij.pants.model.SimpleExportResult.getExportResult(SimpleExportResult.java:90)
	at com.twitter.intellij.pants.util.PantsSdkUtil.createPantsJdk(PantsSdkUtil.java:69)
	at com.twitter.intellij.pants.util.PantsSdkUtil.updateJdk(PantsSdkUtil.java:88)
	at com.twitter.intellij.pants.ui.PantsSdkRefreshAction.actionPerformed(PantsSdkRefreshAction.java:31)
	at com.intellij.openapi.actionSystem.ex.ActionUtil$1.run(ActionUtil.java:266)
	at com.intellij.openapi.actionSystem.ex.ActionUtil.performActionDumbAware(ActionUtil.java:283)
	at com.intellij.openapi.actionSystem.impl.ActionMenuItem$ActionTransmitter.lambda$actionPerformed$0(ActionMenuItem.java:294)
	at com.intellij.openapi.wm.impl.FocusManagerImpl.runOnOwnContext(FocusManagerImpl.java:263)
	at com.intellij.openapi.wm.impl.IdeFocusManagerImpl.runOnOwnContext(IdeFocusManagerImpl.java:77)
	at com.intellij.openapi.actionSystem.impl.ActionMenuItem$ActionTransmitter.actionPerformed(ActionMenuItem.java:284)
	at java.desktop/javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:1967)
	at com.intellij.openapi.actionSystem.impl.ActionMenuItem.lambda$fireActionPerformed$0(ActionMenuItem.java:112)
	at com.intellij.openapi.application.TransactionGuardImpl.runSyncTransaction(TransactionGuardImpl.java:83)
	at com.intellij.openapi.application.TransactionGuardImpl.lambda$submitTransaction$1(TransactionGuardImpl.java:107)
	at com.intellij.openapi.application.TransactionGuardImpl.submitTransaction(TransactionGuardImpl.java:116)
	at com.intellij.openapi.application.TransactionGuard.submitTransaction(TransactionGuard.java:121)
	at com.intellij.openapi.actionSystem.impl.ActionMenuItem.fireActionPerformed(ActionMenuItem.java:112)
	at java.desktop/javax.swing.AbstractButton$Handler.actionPerformed(AbstractButton.java:2308)
	at java.desktop/javax.swing.DefaultButtonModel.fireActionPerformed(DefaultButtonModel.java:405)
	at java.desktop/javax.swing.JToggleButton$ToggleButtonModel.setPressed(JToggleButton.java:401)
	at java.desktop/javax.swing.AbstractButton.doClick(AbstractButton.java:369)
	at java.desktop/com.apple.laf.ScreenMenuItemCheckbox.itemStateChanged(ScreenMenuItemCheckbox.java:198)
	at java.desktop/java.awt.CheckboxMenuItem.processItemEvent(CheckboxMenuItem.java:396)
	at java.desktop/java.awt.CheckboxMenuItem.processEvent(CheckboxMenuItem.java:364)
	at java.desktop/java.awt.MenuComponent.dispatchEventImpl(MenuComponent.java:375)
	at java.desktop/java.awt.MenuComponent.dispatchEvent(MenuComponent.java:363)
	at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:781)
	at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:727)
	at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:95)
	at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:751)
	at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:749)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85)
	at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:748)
	at com.intellij.ide.IdeEventQueue.defaultDispatchEvent(IdeEventQueue.java:906)
	at com.intellij.ide.IdeEventQueue._dispatchEvent(IdeEventQueue.java:779)
	at com.intellij.ide.IdeEventQueue.lambda$dispatchEvent$8(IdeEventQueue.java:422)
	at com.intellij.openapi.progress.impl.CoreProgressManager.computePrioritized(CoreProgressManager.java:698)
	at com.intellij.ide.IdeEventQueue.dispatchEvent(IdeEventQueue.java:421)
	at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

@wisechengyi
Copy link
Collaborator

Would it possible that this does not run on the EDT thread?

  1. Pants -> Invalidate Pants plugin caches
  2. Pants -> refresh pants project sdk, could the exception below because IntelliJ tries to log any work that's blocking the UI thread for a few seconds.
java.lang.Throwable: Synchronous execution on EDT: /Users/yic/workspace/pants/pants --no-quiet export --no-colors --export-output-file=/var/folders/yt/pxdd3gp96530xh_116kskmkh0000gp/T/pants_export_run18005735536549679700.out
	at com.intellij.openapi.diagnostic.Logger.error(Logger.java:145)
	at com.intellij.execution.process.OSProcessHandler.checkEdtAndReadAction(OSProcessHandler.java:147)
	at com.intellij.execution.process.OSProcessHandler.waitFor(OSProcessHandler.java:85)
	at com.intellij.execution.process.CapturingProcessRunner.runProcess(CapturingProcessRunner.java:29)
	at com.intellij.execution.process.CapturingProcessHandler.runProcess(CapturingProcessHandler.java:55)
	at com.twitter.intellij.pants.util.PantsUtil.getCmdOutput(PantsUtil.java:685)
	at com.twitter.intellij.pants.model.SimpleExportResult.getExportResult(SimpleExportResult.java:90)
	at com.twitter.intellij.pants.util.PantsSdkUtil.createPantsJdk(PantsSdkUtil.java:69)
	at com.twitter.intellij.pants.util.PantsSdkUtil.getDefaultJavaSdk(PantsSdkUtil.java:53)
	at com.twitter.intellij.pants.util.PantsSdkUtil.lambda$refreshJdk$8(PantsSdkUtil.java:95)
	at com.intellij.openapi.application.impl.ApplicationImpl.runWriteAction(ApplicationImpl.java:885)
	at com.twitter.intellij.pants.util.PantsSdkUtil.lambda$refreshJdk$9(PantsSdkUtil.java:93)
	at com.intellij.openapi.application.TransactionGuardImpl$2.run(TransactionGuardImpl.java:309)
	at com.intellij.openapi.application.impl.LaterInvocator$1.run(LaterInvocator.java:154)
	at com.intellij.openapi.application.impl.LaterInvocator$FlushQueue.doRun(LaterInvocator.java:441)
	at com.intellij.openapi.application.impl.LaterInvocator$FlushQueue.runNextEvent(LaterInvocator.java:424)
	at com.intellij.openapi.application.impl.LaterInvocator$FlushQueue.run(LaterInvocator.java:407)
	at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:313)
	at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:776)
	at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:727)
	at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85)
	at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:746)
	at com.intellij.ide.IdeEventQueue.defaultDispatchEvent(IdeEventQueue.java:906)
	at com.intellij.ide.IdeEventQueue._dispatchEvent(IdeEventQueue.java:779)
	at com.intellij.ide.IdeEventQueue.lambda$dispatchEvent$8(IdeEventQueue.java:422)
	at com.intellij.openapi.progress.impl.CoreProgressManager.computePrioritized(CoreProgressManager.java:698)
	at com.intellij.ide.IdeEventQueue.dispatchEvent(IdeEventQueue.java:421)
	at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

@mzarnowski
Copy link
Contributor Author

I have missed one spot where a heavy action was running on the UI thread, now it should be good.

Pants -> Invalidate Pants plugin caches

Is your intention here to change the invalidate action to run on non-UI thread? Were there any freezes or errors with the invalidate action?

@wisechengyi
Copy link
Collaborator

invalidate action itself is super quick, so your change should suffice. I will double check when back online.

Copy link
Collaborator

@wisechengyi wisechengyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Marek! would you mind resolve the merge conflict? Then I will merge it in.

mzarnowski and others added 5 commits December 6, 2019 08:58
Previously, the refresh was happening on the EDT thread which
caused errors and freezes because of a process being started inside the
"createPantsJdk" call.

As a result of running in the background, changing the project JDK
(if it wasn't a pants' one) must be scheduled to run in a dispatcher
thread.
Getting the default JDK might take a long time if there is no pants JDK
present and export cache is empty as we have to run an external process.
@wisechengyi wisechengyi merged commit 7335be6 into pantsbuild:master Dec 10, 2019
@mzarnowski mzarnowski deleted the issue/IDE-252/recover-from-incorrect-JDK branch December 17, 2019 17:24
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

Successfully merging this pull request may close these issues.

3 participants