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

arbitrary behavior for export package dialog #534

Closed
gireeshpunathil opened this issue Mar 27, 2023 · 3 comments · Fixed by #567
Closed

arbitrary behavior for export package dialog #534

gireeshpunathil opened this issue Mar 27, 2023 · 3 comments · Fixed by #567

Comments

@gireeshpunathil
Copy link
Contributor

steps to reproduce:

  • create a plugin project
  • open design editor
  • select runtime and click add
  • toggle the Show non-java packages a couple of times
  • repeat this with selecting and deselecting items in the list
  • watch the behavior of Add button

I see arbitrary behaviors for the Add button, w.r.t being enabled / disabled.

@vik-chand
Copy link
Member

Nice catch! That's a bug. I can recreate it too

@gireeshpunathil
Copy link
Contributor Author

relevant code:

private void handleAdd() {
IPluginModelBase model = (IPluginModelBase) getPage().getModel();
final IProject project = model.getUnderlyingResource().getProject();
try {
if (project.hasNature(JavaCore.NATURE_ID)) {
ILabelProvider labelProvider = new JavaElementLabelProvider();
final ConditionalListSelectionDialog dialog = new ConditionalListSelectionDialog(PDEPlugin.getActiveWorkbenchShell(), labelProvider, PDEUIMessages.ExportPackageSection_dialogButtonLabel);
final Collection<String> pckgs = fHeader == null ? Collections.emptySet() : fHeader.getPackageNames();
final boolean allowJava = "true".equals(getBundle().getHeader(ICoreConstants.ECLIPSE_JREBUNDLE)); //$NON-NLS-1$
Runnable runnable = () -> {
ArrayList<IPackageFragment> elements = new ArrayList<>();
ArrayList<IPackageFragment> conditional = new ArrayList<>();
IPackageFragment[] fragments = PDEJavaHelper.getPackageFragments(JavaCore.create(project), pckgs, allowJava);
for (IPackageFragment fragment : fragments) {
try {
if (fragment.containsJavaResources()) {
elements.add(fragment);
} else {
conditional.add(fragment);
}
} catch (JavaModelException e) {
}
}
dialog.setElements(elements.toArray());
dialog.setConditionalElements(conditional.toArray());
dialog.setMultipleSelection(true);
dialog.setMessage(PDEUIMessages.PackageSelectionDialog_label);
dialog.setTitle(PDEUIMessages.ExportPackageSection_title);
dialog.create();
PlatformUI.getWorkbench().getHelpSystem().setHelp(dialog.getShell(), IHelpContextIds.EXPORT_PACKAGES);
SWTUtil.setDialogSize(dialog, 400, 500);
};
BusyIndicator.showWhile(Display.getCurrent(), runnable);
if (dialog.open() == Window.OK) {
Object[] selected = dialog.getResult();
if (fHeader != null) {
for (Object selectedObject : selected) {
IPackageFragment candidate = (IPackageFragment) selectedObject;
fHeader.addPackage(new ExportPackageObject(fHeader, candidate, getVersionAttribute()));
}
} else {
getBundle().setHeader(getExportedPackageHeader(), getValue(selected));
// the way events get triggered, updateButtons isn't called
if (selected.length > 0)
getTablePart().setButtonEnabled(CALCULATE_USE_INDEX, true);
}
}
labelProvider.dispose();
}
} catch (CoreException e) {
}
}

I am unable to locate the code pertinent to the button and its selection change callback, so not sure if this is an in-built behaviour of the ConditionalListSelectionDialog class or not.

@gireeshpunathil
Copy link
Contributor Author

These are at least two threads that participate in this problem, so looks to me like a complex race condition.

FilteredList$TableUpdateJob.runInUIThread(IProgressMonitor) line: 543	
FilteredList$TableUpdateJob(UIJob).lambda$0(IProgressMonitor) line: 148	
0x000000080194e7a0.run() line: not available	
RunnableLock.run(Display) line: 40	
UISynchronizer(Synchronizer).runAsyncMessages(boolean) line: 132	
Display.runAsyncMessages(boolean) line: 4368	
Display.readAndDispatch() line: 3991	
ConditionalListSelectionDialog(Window).runEventLoop(Shell) line: 823	
ConditionalListSelectionDialog(Window).open() line: 799	
ConditionalListSelectionDialog(AbstractElementListSelectionDialog).open() line: 457	
ExportPackageSection.handleAdd() line: 418	
FilteredList.setElements(Object[]) line: 273	
ConditionalListSelectionDialog(AbstractElementListSelectionDialog).setListElements(Object[]) line: 191	
ConditionalListSelectionDialog(ElementListSelectionDialog).createDialogArea(Composite) line: 63	
ConditionalListSelectionDialog.createDialogArea(Composite) line: 43	
ConditionalListSelectionDialog(Dialog).createContents(Composite) line: 767	
ConditionalListSelectionDialog(Window).create() line: 431	
ConditionalListSelectionDialog(Dialog).create() line: 1094	
ConditionalListSelectionDialog(SelectionStatusDialog).create() line: 155	
ConditionalListSelectionDialog(AbstractElementListSelectionDialog).access$superCreate() line: 462	
ConditionalListSelectionDialog(AbstractElementListSelectionDialog).lambda$1() line: 469	

gireeshpunathil added a commit to gireeshpunathil/eclipse.pde that referenced this issue Apr 6, 2023
There are potential race conditions that are happening
between the dialog creation with empty list and the
thread that populates the list and fires selection change
events.

An easy hack is to make sure the button is enabled
whenever there are items in the list. This always
works, as the default behavior of list makes sure
that there is at least one item selected in a
non-empty list.

Fixes: eclipse-pde#534
vik-chand pushed a commit to gireeshpunathil/eclipse.pde that referenced this issue Apr 11, 2023
There are potential race conditions that are happening
between the dialog creation with empty list and the
thread that populates the list and fires selection change
events.

An easy hack is to make sure the button is enabled
whenever there are items in the list. This always
works, as the default behavior of list makes sure
that there is at least one item selected in a
non-empty list.

Fixes: eclipse-pde#534
vik-chand pushed a commit that referenced this issue Apr 11, 2023
* make export dialog add button follow list selection

There are potential race conditions that are happening
between the dialog creation with empty list and the
thread that populates the list and fires selection change
events.

An easy hack is to make sure the button is enabled
whenever there are items in the list. This always
works, as the default behavior of list makes sure
that there is at least one item selected in a
non-empty list.

Fixes: #534

* fixup: address review comments
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 a pull request may close this issue.

2 participants