From 00bc06dc017ebe82b4e8e04d83ccfb237d5d1fdd Mon Sep 17 00:00:00 2001 From: "Ilya.Kazakevich" Date: Thu, 13 Jul 2017 19:21:12 +0300 Subject: [PATCH] PY-21002: Do not access remote servers from AWT thread PY-21002: Do not list interpreter packages under write action We need to access remote server to check if it has Django. We now pretend it has and check only when remote project creation process starts because it runs under progress anyway. PyUtil.runWithProgress executes its task on pool in all cases but when called under write action. In this case AWT thread is used. There is nothing wrong with calling refreshAndGetPackagesModally on AWT unless we use write action because we need to use modal dialog anyway and that is ecactly what this method does. Conclusion : we shall not use refreshAndGetPackagesModally under write action and all other usages are absolutely legal --- .../steps/ProjectSpecificSettingsStep.java | 16 +- .../python/packaging/PyPackageUtil.java | 25 +-- .../PyIntegratedToolsProjectConfigurator.java | 146 +++++++++--------- 3 files changed, 102 insertions(+), 85 deletions(-) diff --git a/python/python-community-configure/src/com/jetbrains/python/newProject/steps/ProjectSpecificSettingsStep.java b/python/python-community-configure/src/com/jetbrains/python/newProject/steps/ProjectSpecificSettingsStep.java index 4ed57ef13e6a..1ad37774a7b6 100644 --- a/python/python-community-configure/src/com/jetbrains/python/newProject/steps/ProjectSpecificSettingsStep.java +++ b/python/python-community-configure/src/com/jetbrains/python/newProject/steps/ProjectSpecificSettingsStep.java @@ -241,6 +241,18 @@ public class ProjectSpecificSettingsStep extends ProjectSettingsStepBase i if (myProjectGenerator instanceof PyFrameworkProjectGenerator) { PyFrameworkProjectGenerator frameworkProjectGenerator = (PyFrameworkProjectGenerator)myProjectGenerator; String frameworkName = frameworkProjectGenerator.getFrameworkTitle(); + + if (isPy3k && !((PyFrameworkProjectGenerator)myProjectGenerator).supportsPython3()) { + setErrorText(frameworkName + " is not supported for the selected interpreter"); + return false; + } + + if (PythonSdkType.isRemote(sdk)) { + return true; + } + // All code beyond this line may be heavy in case of remote sdk and should not be called on AWT + // pretend everything is ok for remote and check package later + if (!isFrameworkInstalled(sdk)) { if (PyPackageUtil.packageManagementEnabled(sdk)) { myInstallFramework = true; @@ -269,10 +281,6 @@ public class ProjectSpecificSettingsStep extends ProjectSettingsStepBase i final String warning = StringUtil.join(warningList, "
"); setWarningText(warning); } - if (isPy3k && !((PyFrameworkProjectGenerator)myProjectGenerator).supportsPython3()) { - setErrorText(frameworkName + " is not supported for the selected interpreter"); - return false; - } } } return true; diff --git a/python/src/com/jetbrains/python/packaging/PyPackageUtil.java b/python/src/com/jetbrains/python/packaging/PyPackageUtil.java index bb64f3f28b2d..0b1b4235cd1a 100644 --- a/python/src/com/jetbrains/python/packaging/PyPackageUtil.java +++ b/python/src/com/jetbrains/python/packaging/PyPackageUtil.java @@ -16,6 +16,7 @@ package com.jetbrains.python.packaging; import com.intellij.execution.ExecutionException; +import com.intellij.openapi.application.Application; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.editor.Document; @@ -45,7 +46,6 @@ import com.jetbrains.python.psi.resolve.QualifiedResolveResult; import com.jetbrains.python.psi.types.TypeEvalContext; import com.jetbrains.python.remote.PyCredentialsContribution; import com.jetbrains.python.sdk.CredentialsTypeExChecker; -import com.jetbrains.python.sdk.PySdkUtil; import com.jetbrains.python.sdk.PythonSdkType; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -71,7 +71,7 @@ public class PyPackageUtil { private static final String INSTALL_REQUIRES = "install_requires"; @NotNull - private static final String[] SETUP_PY_REQUIRES_KWARGS_NAMES = new String[] { + private static final String[] SETUP_PY_REQUIRES_KWARGS_NAMES = new String[]{ REQUIRES, INSTALL_REQUIRES, "setup_requires", "tests_require" }; @@ -284,24 +284,27 @@ public class PyPackageUtil { * Refresh the list of installed packages inside the specified SDK if it hasn't been updated yet * displaying modal progress bar in the process, return cached packages otherwise. *

- * Note that it's unsafe to call this method from a write action AND for for a remote SDK, since such modal - * tasks are executed directly on EDT and network operations on the dispatch thread are prohibited + * Note that you shall never call this method from a write action, since such modal + * tasks are executed directly on EDT and network operations on the dispatch thread are prohibited * (see the implementation of ApplicationImpl#runProcessWithProgressSynchronously() for details). */ @Nullable public static List refreshAndGetPackagesModally(@NotNull Sdk sdk) { + + final Application app = ApplicationManager.getApplication(); + assert !(app.isWriteAccessAllowed()) : + "This method can't be called on WriteAction because " + + "refreshAndGetPackages would be called on AWT thread in this case (see runProcessWithProgressSynchronously) " + + "and may lead to freeze"; + + final Ref> packagesRef = Ref.create(); - final boolean invokedUnderWriteAction = ApplicationManager.getApplication().isWriteAccessAllowed(); @SuppressWarnings("ThrowableInstanceNeverThrown") final Throwable callStacktrace = new Throwable(); LOG.debug("Showing modal progress for collecting installed packages", new Throwable()); PyUtil.runWithProgress(null, PyBundle.message("sdk.scanning.installed.packages"), true, false, indicator -> { indicator.setIndeterminate(true); try { final PyPackageManager manager = PyPackageManager.getInstance(sdk); - if (PySdkUtil.isRemote(sdk) && invokedUnderWriteAction && manager.getPackages() == null) { - LOG.warn("Requesting the installed packages of a remote interpreter using PyPackageUtil#refreshAndGetPackagesModally() " + - "inside a write action is going to cause network operations on EDT"); - } packagesRef.set(manager.refreshAndGetPackages(false)); } catch (ExecutionException e) { @@ -320,7 +323,7 @@ public class PyPackageUtil { /** * Run unconditional update of the list of packages installed in SDK. Normally only one such of updates should run at time. * This behavior in enforced by the parameter isUpdating. - * + * * @param manager package manager for SDK * @param isUpdating flag indicating whether another refresh is already running * @return whether packages were refreshed successfully, e.g. this update wasn't cancelled because of another refresh in progress @@ -343,7 +346,7 @@ public class PyPackageUtil { } return true; } - + @Nullable public static PyPackage findPackage(@NotNull List packages, @NotNull String name) { diff --git a/python/src/com/jetbrains/python/testing/PyIntegratedToolsProjectConfigurator.java b/python/src/com/jetbrains/python/testing/PyIntegratedToolsProjectConfigurator.java index e3637ab8c00e..5f1d1971ec9b 100644 --- a/python/src/com/jetbrains/python/testing/PyIntegratedToolsProjectConfigurator.java +++ b/python/src/com/jetbrains/python/testing/PyIntegratedToolsProjectConfigurator.java @@ -17,13 +17,13 @@ package com.jetbrains.python.testing; import com.intellij.openapi.application.Application; import com.intellij.openapi.application.ApplicationManager; -import com.intellij.openapi.application.ModalityState; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.module.Module; import com.intellij.openapi.module.ModuleManager; import com.intellij.openapi.module.ModuleType; import com.intellij.openapi.project.Project; import com.intellij.openapi.projectRoots.Sdk; +import com.intellij.openapi.util.Computable; import com.intellij.openapi.util.Ref; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.platform.DirectoryProjectConfigurator; @@ -52,7 +52,6 @@ import java.util.concurrent.TimeUnit; /** * Detects test runner and docstring format - * */ public class PyIntegratedToolsProjectConfigurator implements DirectoryProjectConfigurator { private static final Logger LOG = Logger.getInstance(PyIntegratedToolsProjectConfigurator.class); @@ -64,91 +63,98 @@ public class PyIntegratedToolsProjectConfigurator implements DirectoryProjectCon return; } - for (Module m: ModuleManager.getInstance(project).getModules()) { - if (ModuleType.get(m) instanceof PythonModuleTypeBase) { - updateIntegratedTools(m, 10000); + for (final Module module : ModuleManager.getInstance(project).getModules()) { + if (ModuleType.get(module) instanceof PythonModuleTypeBase) { + AppExecutorUtil.getAppScheduledExecutorService().schedule(() -> updateIntegratedTools(module), 10, TimeUnit.SECONDS); break; } } } - private static void updateIntegratedTools(final Module module, final int delay) { - ModalityState modality = ModalityState.current(); + private static void updateIntegratedTools(@NotNull final Module module) { + Application application = ApplicationManager.getApplication(); + assert !application.isDispatchThread() : "This method should not be called on AWT"; + final PyDocumentationSettings docSettings = PyDocumentationSettings.getInstance(module); - AppExecutorUtil.getAppScheduledExecutorService().schedule(() -> ApplicationManager.getApplication().invokeLater(() -> { - LOG.debug("Integrated tools configurator has started"); - if (module.isDisposed()) return; + LOG.debug("Integrated tools configurator has started"); + if (module.isDisposed()) return; - @NotNull DocStringFormat docFormat = DocStringFormat.PLAIN; - //check setup.py - @NotNull String testRunner = detectTestRunnerFromSetupPy(module); - if (!testRunner.isEmpty()) { - LOG.debug("Test runner '" + testRunner + "' was discovered from setup.py in the module '" + module.getModuleFilePath() + "'"); - } + @NotNull DocStringFormat docFormat = DocStringFormat.PLAIN; + //check setup.py + @NotNull String testRunner = detectTestRunnerFromSetupPy(module); + if (!testRunner.isEmpty()) { + LOG.debug("Test runner '" + testRunner + "' was discovered from setup.py in the module '" + module.getModuleFilePath() + "'"); + } - //try to find test_runner import - final String extension = PythonFileType.INSTANCE.getDefaultExtension(); - // Module#getModuleScope() and GlobalSearchScope#getModuleScope() search only in source roots - final GlobalSearchScope searchScope = module.getModuleScope(); - final Collection pyFiles = FilenameIndex.getAllFilesByExt(module.getProject(), extension, searchScope); - for (VirtualFile file : pyFiles) { - if (file.getName().startsWith("test")) { - if (testRunner.isEmpty()) { - testRunner = checkImports(file, module); //find test runner import - if (!testRunner.isEmpty()) { - LOG.debug("Test runner '" + testRunner + "' was detected from imports in the file '" + file.getPath() + "'"); - } - } - } - else if (docFormat == DocStringFormat.PLAIN) { - docFormat = checkDocstring(file, module); // detect docstring type - if (docFormat != DocStringFormat.PLAIN) { - LOG.debug("Docstring format '" + docFormat + "' was detected from content of the file '" + file.getPath() + "'"); - } - } + //try to find test_runner import + final String extension = PythonFileType.INSTANCE.getDefaultExtension(); + // Module#getModuleScope() and GlobalSearchScope#getModuleScope() search only in source roots + final GlobalSearchScope searchScope = module.getModuleScope(); - if (!testRunner.isEmpty() && docFormat != DocStringFormat.PLAIN) { - break; - } - } - // Check test runners available in the module SDK - if (testRunner.isEmpty()) { - //check if installed in sdk - final Sdk sdk = PythonSdkType.findPythonSdk(module); - if (sdk != null && sdk.getSdkType() instanceof PythonSdkType) { - final List packages = PyPackageUtil.refreshAndGetPackagesModally(sdk); - if (packages != null) { - final boolean nose = PyPackageUtil.findPackage(packages, PyNames.NOSE_TEST) != null; - final boolean pytest = PyPackageUtil.findPackage(packages, PyNames.PY_TEST) != null; - if (nose) - testRunner = PythonTestConfigurationsModel.PYTHONS_NOSETEST_NAME; - else if (pytest) - testRunner = PythonTestConfigurationsModel.PY_TEST_NAME; - if (!testRunner.isEmpty()) { - LOG.debug("Test runner '" + testRunner + "' was detected from SDK " + sdk); - } - } - } - } + final Collection pyFiles = application.runReadAction( + (Computable>)() -> FilenameIndex.getAllFilesByExt(module.getProject(), extension, searchScope)); - final TestRunnerService runnerService = TestRunnerService.getInstance(module); - if (runnerService != null) { + + for (VirtualFile file : pyFiles) { + if (file.getName().startsWith("test")) { if (testRunner.isEmpty()) { - runnerService.setProjectConfiguration(PythonTestConfigurationsModel.PYTHONS_UNITTEST_NAME); + testRunner = checkImports(file, module); //find test runner import + if (!testRunner.isEmpty()) { + LOG.debug("Test runner '" + testRunner + "' was detected from imports in the file '" + file.getPath() + "'"); + } } - else { - runnerService.setProjectConfiguration(testRunner); - LOG.info("Test runner '" + testRunner + "' was detected by project configurator"); + } + else if (docFormat == DocStringFormat.PLAIN) { + docFormat = checkDocstring(file, module); // detect docstring type + if (docFormat != DocStringFormat.PLAIN) { + LOG.debug("Docstring format '" + docFormat + "' was detected from content of the file '" + file.getPath() + "'"); } } - // Documentation settings should have meaningful default already - if (docFormat != DocStringFormat.PLAIN) { - docSettings.setFormat(docFormat); - LOG.info("Docstring format '" + docFormat + "' was detected by project configurator"); + if (!testRunner.isEmpty() && docFormat != DocStringFormat.PLAIN) { + break; } - }, modality), delay, TimeUnit.MILLISECONDS); + } + + // Check test runners available in the module SDK + if (testRunner.isEmpty()) { + //check if installed in sdk + final Sdk sdk = PythonSdkType.findPythonSdk(module); + if (sdk != null && sdk.getSdkType() instanceof PythonSdkType) { + final List packages = PyPackageUtil.refreshAndGetPackagesModally(sdk); + if (packages != null) { + final boolean nose = PyPackageUtil.findPackage(packages, PyNames.NOSE_TEST) != null; + final boolean pytest = PyPackageUtil.findPackage(packages, PyNames.PY_TEST) != null; + if (nose) { + testRunner = PythonTestConfigurationsModel.PYTHONS_NOSETEST_NAME; + } + else if (pytest) { + testRunner = PythonTestConfigurationsModel.PY_TEST_NAME; + } + if (!testRunner.isEmpty()) { + LOG.debug("Test runner '" + testRunner + "' was detected from SDK " + sdk); + } + } + } + } + + final TestRunnerService runnerService = TestRunnerService.getInstance(module); + if (runnerService != null) { + if (testRunner.isEmpty()) { + runnerService.setProjectConfiguration(PythonTestConfigurationsModel.PYTHONS_UNITTEST_NAME); + } + else { + runnerService.setProjectConfiguration(testRunner); + LOG.info("Test runner '" + testRunner + "' was detected by project configurator"); + } + } + + // Documentation settings should have meaningful default already + if (docFormat != DocStringFormat.PLAIN) { + docSettings.setFormat(docFormat); + LOG.info("Docstring format '" + docFormat + "' was detected by project configurator"); + } } @NotNull