Changeset 19085 in josm


Ignore:
Timestamp:
2024-05-22T21:12:24+02:00 (5 weeks ago)
Author:
taylor.smock
Message:

Fix #23687: Freeze when opening Add Tag dialog

This occurs when the following are true:

  • The EDT is in a blocking dialog where a DataSet write lock is held
  • The EDT decides to repaint the mapview
  • The style caching occurs on a different thread

Why is this a problem?
The style caching waits for a read lock in a different thread, which will never
happen since the EDT has a write lock. The try-catch block that checks to see if
the current thread can get a read lock does not work on a thread that already has
a write lock, since that thread will always be able to obtain a read lock.

There are two solutions to this:

  1. Change the style caching code such that it times out after some period of time and then retries in the EDT (to avoid recurrent timeouts)
  2. Change TagEditHelper such that the blocking dialog is not in a block which holds a DataSet write lock


Option 2 is better, since it fixes the root cause; option 1 may need to be done
in the future, if only to allow for bug reports.

File:
1 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/gui/dialogs/properties/TagEditHelper.java

    r19006 r19085  
    141141    /** The preference storage of recent tags */
    142142    public static final ListProperty PROPERTY_RECENT_TAGS = new ListProperty("properties.recent-tags",
    143             Collections.<String>emptyList());
     143            Collections.emptyList());
    144144    /** The preference list of tags which should not be remembered, since r9940 */
    145145    public static final StringProperty PROPERTY_TAGS_TO_IGNORE = new StringProperty("properties.recent-tags.ignore",
     
    181181    /**
    182182     * Copy of recently added tags in sorted from newest to oldest order.
    183      *
     183     * <p>
    184184     * We store the maximum number of recent tags to allow dynamic change of number of tags shown in the preferences.
    185185     * Used to cache initial status.
     
    278278        if (activeDataSet == null)
    279279            return;
    280         try {
    281             activeDataSet.beginUpdate();
    282             Collection<OsmPrimitive> selection = OsmDataManager.getInstance().getInProgressSelection();
    283             this.sel = selection;
    284             if (Utils.isEmpty(selection))
    285                 return;
    286 
    287             final AddTagsDialog addDialog = getAddTagsDialog();
    288 
    289             addDialog.showDialog();
    290 
    291             addDialog.destroyActions();
     280        final Collection<OsmPrimitive> selection = updateSelection();
     281
     282        if (Utils.isEmpty(selection))
     283            return;
     284
     285        final AddTagsDialog addDialog = getAddTagsDialog();
     286
     287        addDialog.showDialog();
     288
     289        addDialog.destroyActions();
     290        activeDataSet.update(() -> {
    292291            // Remote control can cause the selection to change, see #23191.
    293             if (addDialog.getValue() == 1 && (selection == sel || warnSelectionChanged())) {
     292            if (addDialog.getValue() == 1 && (selection.equals(updateSelection()) || warnSelectionChanged())) {
    294293                addDialog.performTagAdding(selection);
    295294            } else {
    296295                addDialog.undoAllTagsAdding();
    297296            }
    298         } finally {
    299             activeDataSet.endUpdate();
    300         }
     297        });
    301298    }
    302299
     
    317314    public void editTag(final int row, boolean focusOnKey) {
    318315        changedKey = null;
    319         sel = OsmDataManager.getInstance().getInProgressSelection();
     316        updateSelection();
    320317        if (Utils.isEmpty(sel))
    321318            return;
     
    361358
    362359    /**
     360     * Update the current selection for this editor
     361     */
     362    private Collection<OsmPrimitive> updateSelection() {
     363        final DataSet activeDataSet = OsmDataManager.getInstance().getActiveDataSet();
     364        try {
     365            activeDataSet.getReadLock().lock();
     366            Collection<OsmPrimitive> selection = new ArrayList<>(OsmDataManager.getInstance().getInProgressSelection());
     367            this.sel = selection;
     368            return selection;
     369        } finally {
     370            activeDataSet.getReadLock().unlock();
     371        }
     372    }
     373
     374    /**
    363375     * For a given key k, return a list of keys which are used as keys for
    364376     * auto-completing values to increase the search space.
     
    370382            return Arrays.asList("addr:street", "name");
    371383        else
    372             return Arrays.asList(key);
     384            return Collections.singletonList(key);
    373385    }
    374386
     
    552564            p.add(new JLabel(tr("Key")), GBC.std());
    553565            p.add(Box.createHorizontalStrut(10), GBC.std());
    554             p.add(keys, GBC.eol().fill(GBC.HORIZONTAL));
     566            p.add(keys, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
    555567
    556568            List<AutoCompletionItem> valueList = autocomplete.getTagValues(getAutocompletionKeys(key), usedValuesAwareComparator);
     
    569581            p.add(new JLabel(tr("Value")), GBC.std());
    570582            p.add(Box.createHorizontalStrut(10), GBC.std());
    571             p.add(values, GBC.eol().fill(GBC.HORIZONTAL));
     583            p.add(values, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
    572584            p.add(Box.createVerticalStrut(2), GBC.eol());
    573585
     
    829841            mainPanel.add(new JLabel("<html>"+trn("This will change up to {0} object.",
    830842                "This will change up to {0} objects.", sel.size(), sel.size())
    831                 +"<br><br>"+tr("Please select a key")), GBC.eol().fill(GBC.HORIZONTAL));
     843                +"<br><br>"+tr("Please select a key")), GBC.eol().fill(GridBagConstraints.HORIZONTAL));
    832844
    833845            keys = new AutoCompComboBox<>();
     
    837849            keys.setAutocompleteEnabled(AUTOCOMPLETE_KEYS.get());
    838850
    839             mainPanel.add(keys, GBC.eop().fill(GBC.HORIZONTAL));
     851            mainPanel.add(keys, GBC.eop().fill(GridBagConstraints.HORIZONTAL));
    840852            mainPanel.add(new JLabel(tr("Choose a value")), GBC.eol());
    841853
     
    846858            values.setAutocompleteEnabled(AUTOCOMPLETE_VALUES.get());
    847859
    848             mainPanel.add(values, GBC.eop().fill(GBC.HORIZONTAL));
     860            mainPanel.add(values, GBC.eop().fill(GridBagConstraints.HORIZONTAL));
    849861
    850862            cacheRecentTags();
     
    978990                    lines.add(sc.getKeyText() + ' ' + tr("to apply first suggestion"))
    979991            );
    980             lines.add(Shortcut.getKeyText(KeyStroke.getKeyStroke(KeyEvent.VK_ENTER, KeyEvent.SHIFT_DOWN_MASK)) + ' '
     992            lines.add(Shortcut.getKeyText(KeyStroke.getKeyStroke(KeyEvent.VK_ENTER, InputEvent.SHIFT_DOWN_MASK)) + ' '
    981993                    +tr("to add without closing the dialog"));
    982             Shortcut.findShortcut(KeyEvent.VK_1, commandDownMask | KeyEvent.SHIFT_DOWN_MASK).ifPresent(sc ->
     994            Shortcut.findShortcut(KeyEvent.VK_1, commandDownMask | InputEvent.SHIFT_DOWN_MASK).ifPresent(sc ->
    983995                    lines.add(sc.getKeyText() + ' ' + tr("to add first suggestion without closing the dialog"))
    984996            );
     
    10131025                recentTagsPanel = new JPanel(new GridBagLayout());
    10141026                buildRecentTagsPanel();
    1015                 mainPanel.add(recentTagsPanel, GBC.eol().fill(GBC.HORIZONTAL));
     1027                mainPanel.add(recentTagsPanel, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
    10161028            } else {
    10171029                Dimension panelOldSize = recentTagsPanel.getPreferredSize();
     
    11321144                JPanel tagPanel = new JPanel(new FlowLayout(FlowLayout.LEFT, 0, 0));
    11331145                tagPanel.add(tagLabel);
    1134                 recentTagsPanel.add(tagPanel, GBC.eol().fill(GBC.HORIZONTAL));
     1146                recentTagsPanel.add(tagPanel, GBC.eol().fill(GridBagConstraints.HORIZONTAL));
    11351147            }
    11361148            // Clear label if no tags were added
Note: See TracChangeset for help on using the changeset viewer.