diff -r 6fa0d51b7a56 db/nbproject/project.xml --- a/db/nbproject/project.xml Mon Dec 19 07:32:04 2011 +0100 +++ b/db/nbproject/project.xml Mon Dec 19 23:08:15 2011 +0100 @@ -90,7 +90,7 @@ - 1.1 + 1.10 diff -r 6fa0d51b7a56 db/src/org/netbeans/modules/db/explorer/Bundle.properties --- a/db/src/org/netbeans/modules/db/explorer/Bundle.properties Mon Dec 19 07:32:04 2011 +0100 +++ b/db/src/org/netbeans/modules/db/explorer/Bundle.properties Mon Dec 19 23:08:15 2011 +0100 @@ -123,6 +123,4 @@ DbExtendedDelete_ConfirmationTitle_Others={0,choice,1#Object|1 - 1.5 + 1.10 diff -r 6fa0d51b7a56 j2eeserver/src/org/netbeans/modules/j2ee/deployment/impl/Bundle.properties --- a/j2eeserver/src/org/netbeans/modules/j2ee/deployment/impl/Bundle.properties Mon Dec 19 07:32:04 2011 +0100 +++ b/j2eeserver/src/org/netbeans/modules/j2ee/deployment/impl/Bundle.properties Mon Dec 19 23:08:15 2011 +0100 @@ -206,7 +206,6 @@ MSG_KeyringDefaultDisplayName=Java EE server password MSG_KeyringDisplayName={0} passsword -MSG_KeyringAccess=Requesting keyring access # Options export J2EE.Options.Export.displayName=Java EE diff -r 6fa0d51b7a56 j2eeserver/src/org/netbeans/modules/j2ee/deployment/impl/ServerRegistry.java --- a/j2eeserver/src/org/netbeans/modules/j2ee/deployment/impl/ServerRegistry.java Mon Dec 19 07:32:04 2011 +0100 +++ b/j2eeserver/src/org/netbeans/modules/j2ee/deployment/impl/ServerRegistry.java Mon Dec 19 23:08:15 2011 +0100 @@ -46,8 +46,6 @@ import java.io.File; import java.io.IOException; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeoutException; import java.util.logging.Logger; import org.netbeans.modules.j2ee.deployment.devmodules.api.J2eeModule; import org.netbeans.modules.j2ee.deployment.plugins.api.InstanceProperties; @@ -67,17 +65,12 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.Callable; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; import java.util.logging.Level; -import javax.swing.SwingUtilities; import org.netbeans.api.annotations.common.CheckForNull; import org.netbeans.api.annotations.common.NonNull; import org.netbeans.api.annotations.common.NullAllowed; import org.netbeans.api.keyring.Keyring; -import org.netbeans.api.progress.ProgressUtils; import org.netbeans.modules.j2ee.deployment.devmodules.spi.InstanceListener; import org.netbeans.modules.j2ee.deployment.plugins.api.AlreadyRegisteredException; import org.netbeans.modules.j2ee.deployment.plugins.spi.OptionalDeploymentManagerFactory; @@ -87,7 +80,6 @@ import org.openide.filesystems.FileEvent; import org.openide.filesystems.FileObject; import org.openide.filesystems.FileUtil; -import org.openide.util.RequestProcessor; public final class ServerRegistry implements java.io.Serializable { @@ -99,8 +91,6 @@ public static final String TARGETNAME_ATTR = "targetName"; //NOI18N public static final String SERVER_NAME = "serverName"; //NOI18N - private static final RequestProcessor KEYRING_ACCESS = new RequestProcessor(); - private static ServerRegistry instance = null; public synchronized static ServerRegistry getInstance() { if(instance == null) instance = new ServerRegistry(); @@ -424,12 +414,8 @@ LOGGER.log(Level.INFO, null, ioe); } } - KEYRING_ACCESS.post(new Runnable() { - @Override - public void run() { - Keyring.delete(getPasswordKey(url)); - } - }); + + Keyring.delete(getPasswordKey(url)); } /** @@ -651,97 +637,44 @@ public static Profiler getProfiler() { return (Profiler)Lookup.getDefault().lookup(Profiler.class); } - + @CheckForNull static String readPassword(@NonNull final String url) { - Callable call = new Callable() { - @Override - public String call() throws Exception { - char[] passwordChars = Keyring.read(getPasswordKey(url)); - if (passwordChars != null) { - String password = String.valueOf(passwordChars); - Arrays.fill(passwordChars, ' '); - return password; - } - return null; - } - }; - return readPassword(call); + char[] passwordChars = Keyring.read(getPasswordKey(url)); + if (passwordChars != null) { + String password = String.valueOf(passwordChars); + Arrays.fill(passwordChars, ' '); + return password; + } + return null; } - + static void savePassword(@NonNull final String url, @NullAllowed final String password, @NullAllowed final String displayName) { - - Runnable run = new Runnable() { - @Override - public void run() { - if (password == null) { - return; - } - Keyring.save(getPasswordKey(url), password.toCharArray(), displayName); - } - }; - KEYRING_ACCESS.post(run); + + if (password == null) { + return; + } + Keyring.save(getPasswordKey(url), password.toCharArray(), displayName); } static void savePassword(@NonNull final FileObject fo, @NullAllowed final String password, @NullAllowed final String displayName) { - Runnable run = new Runnable() { - - @Override - public void run() { - if (password == null) { - return; - } - String url = (String) fo.getAttribute(InstanceProperties.URL_ATTR); - if (url == null) { - return; - } - Keyring.save(getPasswordKey(url), password.toCharArray(), displayName); - try { - fo.setAttribute(InstanceProperties.PASSWORD_ATTR, null); - } catch (IOException ex) { - LOGGER.log(Level.INFO, null, ex); - } - } - }; - KEYRING_ACCESS.post(run); - } - - private static String readPassword(Callable readTask) { + if (password == null) { + return; + } + String url = (String) fo.getAttribute(InstanceProperties.URL_ATTR); + if (url == null) { + return; + } + Keyring.save(getPasswordKey(url), password.toCharArray(), displayName); try { - final Future result = KEYRING_ACCESS.submit(readTask); - if (SwingUtilities.isEventDispatchThread()) { - if (!result.isDone()) { - try { - // lets wait in awt to avoid flashing dialogs - result.get(50, TimeUnit.MILLISECONDS); - } catch (TimeoutException ex) { - ProgressUtils.showProgressDialogAndRun(new Runnable() { - - @Override - public void run() { - try { - result.get(); - } catch (InterruptedException ex) { - Thread.currentThread().interrupt(); - } catch (ExecutionException ex) { - LOGGER.log(Level.INFO, null, ex); - } - } - }, NbBundle.getMessage(ServerRegistry.class, "MSG_KeyringAccess")); - } - } - } - return result.get(); - } catch (InterruptedException ex) { - Thread.currentThread().interrupt(); - } catch (ExecutionException ex) { + fo.setAttribute(InstanceProperties.PASSWORD_ATTR, null); + } catch (IOException ex) { LOGGER.log(Level.INFO, null, ex); } - return null; - } + } private static String getPasswordKey(String url) { StringBuilder builder = new StringBuilder("j2eeserver:"); diff -r 6fa0d51b7a56 j2eeserver/test/unit/src/org/netbeans/modules/j2ee/deployment/plugins/api/InstancePropertiesTest.java --- a/j2eeserver/test/unit/src/org/netbeans/modules/j2ee/deployment/plugins/api/InstancePropertiesTest.java Mon Dec 19 07:32:04 2011 +0100 +++ b/j2eeserver/test/unit/src/org/netbeans/modules/j2ee/deployment/plugins/api/InstancePropertiesTest.java Mon Dec 19 23:08:15 2011 +0100 @@ -200,13 +200,13 @@ public void testPasswordInKeyring() throws Exception { // the instance from test layer - assertEquals("Adminpasswd", getPasswordFromKeyring("j2eeserver:fooservice")); + assertEquals("Adminpasswd", new String(Keyring.read("j2eeserver:fooservice"))); // new instance String url = TEST_URL_PREFIX + "passwordInKeyring"; InstanceProperties.createInstanceProperties( url, TEST_USERNAME, TEST_PASSWORD, TEST_DISPLAY_NAME); - assertEquals(TEST_PASSWORD, getPasswordFromKeyring("j2eeserver:" + url)); + assertEquals(TEST_PASSWORD, new String(Keyring.read("j2eeserver:" + url))); // all password attributes are converted to keyring FileObject fo = FileUtil.getConfigFile("J2EE/InstalledServers"); @@ -219,31 +219,10 @@ String url = TEST_URL_PREFIX + "keyringCleanup"; InstanceProperties.createInstanceProperties( url, TEST_USERNAME, TEST_PASSWORD, TEST_DISPLAY_NAME); - assertEquals(TEST_PASSWORD, getPasswordFromKeyring("j2eeserver:" + url)); + assertEquals(TEST_PASSWORD, new String(Keyring.read("j2eeserver:" + url))); ServerRegistry.getInstance().removeServerInstance(url); - assertNull(getPasswordFromKeyring("j2eeserver:" + url)); - } - - private static String getPasswordFromKeyring(final String key) throws Exception { - Future ret = getKeyringAccess().submit(new Callable() { - - @Override - public String call() throws Exception { - char[] passwd = Keyring.read(key); - if (passwd != null) { - return String.valueOf(passwd); - } - return null; - } - }); - return ret.get(5, TimeUnit.SECONDS); - } - - private static RequestProcessor getKeyringAccess() throws Exception { - Field field = ServerRegistry.class.getDeclaredField("KEYRING_ACCESS"); - field.setAccessible(true); - return (RequestProcessor) field.get(null); + assertNull(Keyring.read("j2eeserver:" + url)); } private static void assertPropertiesEquals(Map expected, InstanceProperties props) { diff -r 6fa0d51b7a56 keyring/apichanges.xml --- a/keyring/apichanges.xml Mon Dec 19 07:32:04 2011 +0100 +++ b/keyring/apichanges.xml Mon Dec 19 23:08:15 2011 +0100 @@ -49,6 +49,35 @@ Keyring API + + + Keyring API usable from any thread + + + + +

+ Existing callers should use the Keyring directly from + any thread and should not try to avoid EDT anymore. +

+

+ SPI implementors should not be changed and they may + continue to assume that they will not be called directly + from EDT. +

+
+ +

+ It hasn't been allowed to call the Keyring from EDT. + This change removes the limitation as the need to read + password from the UI is not so rare. To resolve this people + had to code custom threading solution to prevent possible + deadlock on fallback implementation of the keyring API. +

+
+ + +
Keyring API created diff -r 6fa0d51b7a56 keyring/arch.xml --- a/keyring/arch.xml Mon Dec 19 07:32:04 2011 +0100 +++ b/keyring/arch.xml Mon Dec 19 23:08:15 2011 +0100 @@ -612,7 +612,8 @@ -->

- XXX no answer for exec-threading + The API methods may be called from any thread. The SPI implementors may + assume that they will not be called directly from EDT.

diff -r 6fa0d51b7a56 keyring/manifest.mf --- a/keyring/manifest.mf Mon Dec 19 07:32:04 2011 +0100 +++ b/keyring/manifest.mf Mon Dec 19 23:08:15 2011 +0100 @@ -1,6 +1,6 @@ Manifest-Version: 1.0 OpenIDE-Module: org.netbeans.modules.keyring OpenIDE-Module-Localizing-Bundle: org/netbeans/modules/keyring/Bundle.properties -OpenIDE-Module-Specification-Version: 1.9 +OpenIDE-Module-Specification-Version: 1.10 OpenIDE-Module-Recommends: org.netbeans.modules.keyring.impl diff -r 6fa0d51b7a56 keyring/nbproject/project.xml --- a/keyring/nbproject/project.xml Mon Dec 19 07:32:04 2011 +0100 +++ b/keyring/nbproject/project.xml Mon Dec 19 23:08:15 2011 +0100 @@ -6,6 +6,24 @@ org.netbeans.modules.keyring + org.netbeans.api.annotations.common + + + + 1 + 1.13 + + + + org.netbeans.api.progress + + + + 1 + 1.27 + + + org.openide.util diff -r 6fa0d51b7a56 keyring/src/org/netbeans/api/keyring/Keyring.java --- a/keyring/src/org/netbeans/api/keyring/Keyring.java Mon Dec 19 07:32:04 2011 +0100 +++ b/keyring/src/org/netbeans/api/keyring/Keyring.java Mon Dec 19 23:08:15 2011 +0100 @@ -45,21 +45,39 @@ import java.util.Arrays; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.logging.Level; import java.util.logging.Logger; +import javax.swing.SwingUtilities; +import org.netbeans.api.annotations.common.CheckForNull; +import org.netbeans.api.annotations.common.NonNull; +import org.netbeans.api.annotations.common.NullAllowed; +import org.netbeans.api.progress.ProgressHandle; +import org.netbeans.api.progress.ProgressUtils; import org.netbeans.spi.keyring.KeyringProvider; +import org.openide.util.Cancellable; import org.openide.util.Lookup; +import org.openide.util.NbBundle; import org.openide.util.Parameters; +import org.openide.util.RequestProcessor; /** * Client class for working with stored keys (such as passwords). *

The key identifier should be unique for the whole application, * so qualify it with any prefixes as needed. - *

Avoid calling methods on this class from the event dispatch thread, - * as some provider implementations may need to block while displaying a dialog - * (e.g. prompting for a master password to access the keyring). + *

Since 1.10 it is allowed to call methods of this class from even + * dispatch thread. */ -public class Keyring { +public final class Keyring { + + // throughput 1 is intentional + private static final RequestProcessor KEYRING_ACCESS = new RequestProcessor(Keyring.class); + + private static final long SAFE_DELAY = 70; private Keyring() {} @@ -82,43 +100,118 @@ return PROVIDER; } + private static synchronized char[] readImpl(String key) { + LOG.log(Level.FINEST, "reading: {0}", key); + return provider().read(key); + } + /** * Reads a key from the ring. + *

+ * This method can be called from any thread. + * All the changes done by previous calls to {@link #delete(java.lang.String)} + * or {@link #save(java.lang.String, char[], java.lang.String)} methods + * are guaranteed to be visible by subsequent calls to this method. + * * @param key the identifier of the key * @return its value if found (you may null out its elements), else null if not present */ - public static synchronized char[] read(String key) { + @NbBundle.Messages("MSG_KeyringAccess=Requesting keyring access") + @CheckForNull + public static char[] read(@NonNull final String key) { Parameters.notNull("key", key); - LOG.log(Level.FINEST, "reading: {0}", key); - return provider().read(key); + + try { + final Future result = KEYRING_ACCESS.submit(new Callable() { + @Override + public char[] call() throws Exception { + return Keyring.readImpl(key); + } + }); + + if (SwingUtilities.isEventDispatchThread()) { + if (!result.isDone()) { + try { + // lets wait in awt to avoid flashing dialogs + return result.get(SAFE_DELAY, TimeUnit.MILLISECONDS); + } catch (TimeoutException ex) { + // show progress dialog + return ProgressUtils.showProgressDialogAndRun( + new ProgressRunnable(result), Bundle.MSG_KeyringAccess(), false); + } + } + } + return result.get(); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + } catch (ExecutionException ex) { + LOG.log(Level.INFO, null, ex); + } + return null; + } + + private static synchronized void saveImpl(String key, char[] password, String description) { + LOG.log(Level.FINEST, "saving: {0}", key); + provider().save(key, password, description); + Arrays.fill(password, (char) 0); } /** * Saves a key to the ring. * If it could not be saved, does nothing. * If the key already existed, overwrites the password. + *

+ * This method can be called from any thread. + * The changes done by multiple calls to {@link #delete(java.lang.String)} + * or {@link #save(java.lang.String, char[], java.lang.String)} methods + * are guaranteed to be processed in order in which they were called. + * * @param key a key identifier * @param password the password or other sensitive information associated with the key * (its contents will be nulled out by end of call) * @param description a user-visible description of the key (may be null) */ - public static synchronized void save(String key, char[] password, String description) { + public static void save(@NonNull final String key, @NonNull final char[] password, + @NullAllowed final String description) { + Parameters.notNull("key", key); Parameters.notNull("password", password); - LOG.log(Level.FINEST, "saving: {0}", key); - provider().save(key, password, description); - Arrays.fill(password, (char) 0); + + KEYRING_ACCESS.post(new Runnable() { + + @Override + public void run() { + Keyring.saveImpl(key, password, description); + } + }); + } + + private static synchronized void deleteImpl(String key) { + LOG.log(Level.FINEST, "deleting: {0}", key); + provider().delete(key); } /** * Deletes a key from the ring. * If the key was not in the ring to begin with, does nothing. + *

+ * This method can be called from any thread. + * The changes done by multiple calls to {@link #delete(java.lang.String)} + * or {@link #save(java.lang.String, char[], java.lang.String)} methods + * are guaranteed to be processed in order in which they were called. + * * @param key a key identifier */ - public static synchronized void delete(String key) { + public static void delete(@NonNull final String key) { Parameters.notNull("key", key); - LOG.log(Level.FINEST, "deleting: {0}", key); - provider().delete(key); + + KEYRING_ACCESS.post(new Runnable() { + + @Override + public void run() { + Keyring.deleteImpl(key); + } + }); } private static class DummyKeyringProvider implements KeyringProvider { @@ -156,4 +249,29 @@ return result; } + private static class ProgressRunnable implements org.netbeans.api.progress.ProgressRunnable, Cancellable { + + private final Future task; + + public ProgressRunnable(Future task) { + this.task = task; + } + + @Override + public T run(ProgressHandle handle) { + try { + return task.get(); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + } catch (ExecutionException ex) { + LOG.log(Level.INFO, null, ex); + } + return null; + } + + @Override + public boolean cancel() { + return task.cancel(true); + } + } } diff -r 6fa0d51b7a56 php.project/nbproject/project.xml --- a/php.project/nbproject/project.xml Mon Dec 19 07:32:04 2011 +0100 +++ b/php.project/nbproject/project.xml Mon Dec 19 23:08:15 2011 +0100 @@ -177,7 +177,7 @@ - 1.0 + 1.10 diff -r 6fa0d51b7a56 php.project/src/org/netbeans/modules/php/project/connections/spi/Bundle.properties --- a/php.project/src/org/netbeans/modules/php/project/connections/spi/Bundle.properties Mon Dec 19 07:32:04 2011 +0100 +++ b/php.project/src/org/netbeans/modules/php/project/connections/spi/Bundle.properties Mon Dec 19 23:08:15 2011 +0100 @@ -42,4 +42,3 @@ # 0 - name of configuration # 1 - type of a configuration (FTP etc.) MSG_PasswordFor=Password for {0} [{1}] -MSG_KeyringAccess=Requesting keyring access diff -r 6fa0d51b7a56 php.project/src/org/netbeans/modules/php/project/connections/spi/RemoteConfiguration.java --- a/php.project/src/org/netbeans/modules/php/project/connections/spi/RemoteConfiguration.java Mon Dec 19 07:32:04 2011 +0100 +++ b/php.project/src/org/netbeans/modules/php/project/connections/spi/RemoteConfiguration.java Mon Dec 19 23:08:15 2011 +0100 @@ -42,21 +42,13 @@ package org.netbeans.modules.php.project.connections.spi; -import java.util.concurrent.Callable; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import java.util.logging.Level; import java.util.logging.Logger; -import javax.swing.SwingUtilities; import org.netbeans.api.keyring.Keyring; -import org.netbeans.api.progress.ProgressUtils; import org.netbeans.modules.php.api.util.StringUtils; import org.netbeans.modules.php.project.connections.ConfigManager; import org.netbeans.modules.php.project.util.PhpProjectUtils; import org.openide.util.NbBundle; -import org.openide.util.RequestProcessor; /** * Class representing a remote configuration (e.g. FTP, SFTP). @@ -69,7 +61,6 @@ public abstract class RemoteConfiguration { protected static final Logger LOGGER = Logger.getLogger(RemoteConfiguration.class.getName()); - static final RequestProcessor KEYRING_ACCESS = new RequestProcessor(); protected final ConfigManager.Configuration cfg; @@ -301,49 +292,15 @@ } private String readPasswordFromKeyring() { - try { - final Future result = KEYRING_ACCESS.submit(new Callable() { - @Override - public String call() throws Exception { - // new password key - char[] newPassword = Keyring.read(passwordKey); - if (newPassword != null) { - return new String(newPassword); - } - // deprecated password key - newPassword = Keyring.read(deprecatedPasswordKey); - if (newPassword != null) { - return new String(newPassword); - } - return null; - } - }); - if (SwingUtilities.isEventDispatchThread()) { - if (!result.isDone()) { - try { - // let's wait in awt to avoid flashing dialogs - result.get(99, TimeUnit.MILLISECONDS); - } catch (TimeoutException ex) { - ProgressUtils.showProgressDialogAndRun(new Runnable() { - @Override - public void run() { - try { - result.get(); - } catch (InterruptedException ex) { - Thread.currentThread().interrupt(); - } catch (ExecutionException ex) { - LOGGER.log(Level.INFO, null, ex); - } - } - }, NbBundle.getMessage(RemoteConfiguration.class, "MSG_KeyringAccess")); - } - } - } - return result.get(); - } catch (InterruptedException ex) { - Thread.currentThread().interrupt(); - } catch (ExecutionException ex) { - LOGGER.log(Level.INFO, null, ex); + // new password key + char[] newPassword = Keyring.read(passwordKey); + if (newPassword != null) { + return new String(newPassword); + } + // deprecated password key + newPassword = Keyring.read(deprecatedPasswordKey); + if (newPassword != null) { + return new String(newPassword); } return null; } @@ -355,29 +312,19 @@ return; } if (StringUtils.hasText(password)) { - KEYRING_ACCESS.post(new Runnable() { - @Override - public void run() { - Keyring.save(passwordKey, password.toCharArray(), - NbBundle.getMessage(RemoteConfiguration.class, "MSG_PasswordFor", getDisplayName(), type)); - // remove old password key - Keyring.delete(deprecatedPasswordKey); - } - }); + Keyring.save(passwordKey, password.toCharArray(), + NbBundle.getMessage(RemoteConfiguration.class, "MSG_PasswordFor", getDisplayName(), type)); + // remove old password key + Keyring.delete(deprecatedPasswordKey); } else { deletePassword(); } } protected void deletePassword() { - KEYRING_ACCESS.post(new Runnable() { - @Override - public void run() { - Keyring.delete(passwordKey); - // remove old password key - Keyring.delete(deprecatedPasswordKey); - } - }); + Keyring.delete(passwordKey); + // remove old password key + Keyring.delete(deprecatedPasswordKey); } }