Skip to content

Commit

Permalink
Fix NPE during project group creation caused by ergonomics
Browse files Browse the repository at this point in the history 
Projects can change class type due to ergonomics. Hashcode/equals
matching problems make storage in sets/maps problematic.

 - try to unbox the project before use.
 - simplify code paths affected by this
  • Loading branch information
@mbien
mbien committed Jan 20, 2025
1 parent 235b99a commit 8cff047
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 42 deletions.
8 changes: 3 additions & 5 deletions ide/projectui/src/org/netbeans/modules/project/ui/OpenProjectList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,12 +234,9 @@ public Future<Project[]> openProjectsAPI() {
}

final Project unwrapProject(Project wrap) {
Project[] now = getOpenProjects();

if (wrap instanceof LazyProject) {
LazyProject lp = (LazyProject)wrap;
for (Project p : now) {
if (lp.getProjectDirectory().equals(p.getProjectDirectory())) {
for (Project p : getOpenProjects()) {
if (wrap.getProjectDirectory().equals(p.getProjectDirectory())) {
return p;
}
}
Expand Down Expand Up @@ -1379,6 +1376,7 @@ private boolean doOpenProject(final @NonNull Project p) {
public @Override Boolean run() {
log(Level.FINER, "already opened: {0} ", openProjects);
for (Project existing : openProjects) {
// TODO double tap ???
if (p.equals(existing) || existing.equals(p)) {
alreadyOpen.set(true);
return false;
Expand Down
57 changes: 21 additions & 36 deletions ide/projectui/src/org/netbeans/modules/project/ui/ProjectUtilities.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@
*/
public class ProjectUtilities {

private static final Logger LOG = Logger.getLogger(ProjectUtilities.class.getName());

static final String OPEN_FILES_NS = "http://www.netbeans.org/ns/projectui-open-files/1"; // NOI18N
static final String OPEN_FILES_NS2 = "http://www.netbeans.org/ns/projectui-open-files/2"; // NOI18N
static final String OPEN_FILES_ELEMENT = "open-files"; // NOI18N
Expand Down Expand Up @@ -103,30 +105,23 @@ public boolean open (FileObject fo) {
} else if (o != null) {
o.open();
} else {
ERR.log(Level.INFO, "No EditCookie nor OpenCookie nor Openable for {0}", dobj);
LOG.log(Level.INFO, "No EditCookie nor OpenCookie nor Openable for {0}", dobj);
return false;
}
return true;
}

@Override
public Map<Project,Set<String>> close(final Project[] projects,
final boolean notifyUI) {
final Wrapper wr = new Wrapper();
wr.urls4project = new LinkedHashMap<>();
doClose(projects, notifyUI, wr);
return wr.urls4project;
}

private void doClose(Project[] projects, boolean notifyUI, Wrapper wr) {
public Map<Project, Set<String>> close(Project[] projects, boolean notifyUI) {
Map<Project, Set<String>> project2FilesMap = new LinkedHashMap<>();
List<Project> listOfProjects = Arrays.asList(projects);
for (Project p : listOfProjects) { //#232668 all projects need an entry in the map - to handle projects without files correctly
wr.urls4project.put(p, new LinkedHashSet<>());
project2FilesMap.put(p, new LinkedHashSet<>());
}
Set<DataObject> openFiles = new LinkedHashSet<>();
List<TopComponent> tc2close = new ArrayList<>();

ERR.finer("Closing TCs");
LOG.finer("Closing TCs");
List<TopComponent> openedTC = getOpenedTCs();

for (TopComponent tc : openedTC) {
Expand All @@ -135,28 +130,23 @@ private void doClose(Project[] projects, boolean notifyUI, Wrapper wr) {
if (dobj != null) {
FileObject fobj = dobj.getPrimaryFile();
Project owner = ProjectConvertors.getNonConvertorOwner(fobj);
ERR.log(Level.FINER, "Found {0} owned by {1} in {2} of {3}", new Object[] {fobj, owner, tc.getName(), tc.getClass()});
LOG.log(Level.FINER, "Found {0} owned by {1} in {2} of {3}", new Object[] {fobj, owner, tc.getName(), tc.getClass()});

if (listOfProjects.contains(owner)) {
Set<String> files = project2FilesMap.get(owner);
if (files != null) {
if (notifyUI) {
openFiles.add(dobj);
tc2close.add(tc);
} else if (!dobj.isModified()) {
// when not called from UI, only include TCs that arenot modified
tc2close.add(tc);
}
//#235897 split a single line to detect NPE better
final Set<String> pwnr = wr.urls4project.get(owner);
assert pwnr != null : "Owner project for file:" + fobj + " prj:" + owner;
final FileObject pf = fobj;
assert pf != null;
URL u = pf.toURL();
assert u != null;
String uex = u.toExternalForm();
pwnr.add(uex);
files.add(fobj.toURL().toExternalForm());
} else {
LOG.log(Level.WARNING, "project association lost, project ({1}) might lose an opened file ({2}) on reopen", new Object[] {owner, fobj});
}
} else {
ERR.log(Level.FINE, "#194243: no DataObject in lookup of {0} of {1}", new Object[] {tc.getName(), tc.getClass()});
LOG.log(Level.FINE, "#194243: no DataObject in lookup of {0} of {1}", new Object[] {tc.getName(), tc.getClass()});
}
}
if (notifyUI) {
Expand Down Expand Up @@ -189,9 +179,10 @@ public void run() {
} else {
// signal that close was vetoed
if (!openFiles.isEmpty()) {
wr.urls4project = null;
return null;
}
}
return project2FilesMap;
}

private List<TopComponent> getOpenedTCs() {
Expand All @@ -203,7 +194,7 @@ private List<TopComponent> getOpenedTCs() {
if (!wm.isEditorMode(mode)) {
continue;
}
ERR.log(Level.FINER, "Closing TCs in mode {0}", mode.getName());
LOG.log(Level.FINER, "Closing TCs in mode {0}", mode.getName());
openedTC.addAll(Arrays.asList(wm.getOpenedTopComponents(mode)));
}
};
Expand All @@ -220,12 +211,6 @@ private List<TopComponent> getOpenedTCs() {
}
};

private static class Wrapper {
Map<Project,Set<String>> urls4project;
}

private static final Logger ERR = Logger.getLogger(ProjectUtilities.class.getName());

private ProjectUtilities() {}

public static void selectAndExpandProject( final Project p ) {
Expand Down Expand Up @@ -541,12 +526,12 @@ public static Set<FileObject> openProjectFiles (Project p) {

public static Set<FileObject> openProjectFiles (Project p, Group grp) {
String groupName = grp == null ? null : grp.getName();
ERR.log(Level.FINE, "Trying to open files from {0}...", p);
LOG.log(Level.FINE, "Trying to open files from {0}...", p);

List<String> urls = getOpenFilesUrls(p, groupName);
Set<FileObject> toRet = new LinkedHashSet<>();
for (String url : urls) {
ERR.log(Level.FINE, "Will try to open {0}", url);
LOG.log(Level.FINE, "Will try to open {0}", url);
FileObject fo;
try {
fo = URLMapper.findFileObject (new URL (url));
Expand All @@ -555,13 +540,13 @@ public static Set<FileObject> openProjectFiles (Project p, Group grp) {
continue;
}
if (fo == null || !fo.isValid()) { //check for validity because of issue #238488
ERR.log(Level.FINE, "Could not find {0}", url);
LOG.log(Level.FINE, "Could not find {0}", url);
continue;
}

//#109676
if (ProjectConvertors.getNonConvertorOwner(fo) != p) {
ERR.log(Level.FINE, "File {0} doesn''t belong to project at {1}", new Object[] {url, p.getProjectDirectory().getPath()});
LOG.log(Level.FINE, "File {0} doesn''t belong to project at {1}", new Object[] {url, p.getProjectDirectory().getPath()});
continue;
}

Expand Down
14 changes: 13 additions & 1 deletion ide/projectui/src/org/netbeans/modules/project/ui/groups/Group.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,19 @@ static void open(final Group g, String oldGroupName, boolean isNewGroup, Prefere
h.start(200);
ProjectUtilities.WaitCursor.show();
final OpenProjectList opl = OpenProjectList.getDefault();
Set<Project> oldOpen = new HashSet<Project>(Arrays.asList(opl.getOpenProjects()));

Set<Project> oldOpen = new HashSet<>();
for (Project open : opl.getOpenProjects()) {
// TODO fix this properly, e.g investigate if:
// - getOpenProjects() should only return unboxed projects
// risk: called by public API
// - review/fix the broken hashcode/equals contracts
// risk: code contains hacks which account for this already, e.g if (a.equals(b) || b.equals(a))
// for now: unbox potential fod.FeatureNonProject wrapper since it breaks Sets/Maps due to incompatible hashcode/equals impls
Project real = open.getLookup().lookup(Project.class);
oldOpen.add(real != null ? real : open);
}

//TODO switching to no group always clears the opened project list.
Set<Project> newOpen = g != null ? g.getProjects(h, 10, 100) : getProjectsByPreferences(noneGroupPref, h, 10, 100);
final Set<Project> toClose = new HashSet<Project>(oldOpen);
Expand Down

0 comments on commit 8cff047

Please sign in to comment.