1
0
Fork 0
mirror of https://github.com/eclipse-cdt/cdt synced 2025-08-02 22:05:44 +02:00

Cleaning project fails once the binary is expanded in project explorer on Windows

Steps:
======
1. Create a managed project and build it
2. Expand the built binary available in binary container in project explorer view
3. Now clean the project, clean will fail irrespective of number of tries you do

Reason:
=======
For finding the sources for binary, Elf instance is created and Section.mapSectionData creates MappedByteBuffer of channel which locks the file on Windows until its garbage collected, see following
https://bugs.java.com/bugdatabase/view_bug.do?bug_id=4715154

Solution:
=========
Made ISymbolReader AutoCloseable and user is responsible to properly close it. In case of dwarf reader, we remove all the references of ByteBuffer and call gc.
This commit is contained in:
Umair Sair 2022-11-01 01:35:49 +05:00 committed by Jonah Graham
parent 571d62d6f9
commit b10979677e
19 changed files with 144 additions and 49 deletions

View file

@ -55,6 +55,10 @@ It may have other uses to other API consumers as well and is therefore included
This should allow ISV's to create MBS based project with a vendor-specific build-system ID without using internal API. This should allow ISV's to create MBS based project with a vendor-specific build-system ID without using internal API.
## Binary Parser code uses AutoCloseable
The binary parser classes which open binary files now implement AutoCloseable so they can (and should) be used in a try-with-resources block.
See https://github.com/eclipse-cdt/cdt/pull/132
# Bugs Fixed in this Release # Bugs Fixed in this Release

View file

@ -14,6 +14,7 @@ This section describes API removals that occurred in past releases, and upcoming
- [Removal of deprecated CBuildConfiguration.watchProcess() methods](#watchProcessCBuildConfig) - [Removal of deprecated CBuildConfiguration.watchProcess() methods](#watchProcessCBuildConfig)
- [Rework of API to determine GDB command line in org.eclipse.cdt.dsf.gdb](#gdbBackendDebuggerCommandLine) - [Rework of API to determine GDB command line in org.eclipse.cdt.dsf.gdb](#gdbBackendDebuggerCommandLine)
- [Removal of Qt plug-ins and features](#qt-plugins) - [Removal of Qt plug-ins and features](#qt-plugins)
- [Removal of constructor org.eclipse.cdt.utils.coff.CodeViewReader(RandomAccessFile, int, boolean)](#CodeViewReader-constructor-removal)
## API Changes in CDT 10.5.0 ## API Changes in CDT 10.5.0
@ -163,6 +164,14 @@ The following bundles and all their related API has been removed:
See https://github.com/eclipse-cdt/cdt/issues/123 See https://github.com/eclipse-cdt/cdt/issues/123
### <span id="CodeViewReader-constructor-removal">Removal of constructor org.eclipse.cdt.utils.coff.CodeViewReader(RandomAccessFile, int, boolean)</span>
Same instance of RandomAccessFile was shared between multiple objects which
causes problems in closing it properly. A new constructor is introduced which
accepts filename and opens a RandomAccessFile.
See https://github.com/eclipse-cdt/cdt/pull/132
--- ---
## API Changes in CDT 10.5.0. ## API Changes in CDT 10.5.0.

View file

@ -306,11 +306,14 @@ public class Binary extends Openable implements IBinary {
// Try to get the list of source files used to build the binary from the // Try to get the list of source files used to build the binary from the
// symbol information. // symbol information.
ISymbolReader symbolreader = obj.getAdapter(ISymbolReader.class); String[] sourceFiles = null;
try (ISymbolReader symbolreader = obj.getAdapter(ISymbolReader.class)) {
if (symbolreader == null) if (symbolreader == null)
return false; return false;
String[] sourceFiles = symbolreader.getSourceFiles(); sourceFiles = symbolreader.getSourceFiles();
}
if (sourceFiles != null && sourceFiles.length > 0) { if (sourceFiles != null && sourceFiles.length > 0) {
ISourceFinder srcFinder = getAdapter(ISourceFinder.class); ISourceFinder srcFinder = getAdapter(ISourceFinder.class);
try { try {

View file

@ -23,7 +23,7 @@ import org.eclipse.core.runtime.IProgressMonitor;
* @noextend This interface is not intended to be extended by clients. * @noextend This interface is not intended to be extended by clients.
* @noimplement This interface is not intended to be implemented by clients. * @noimplement This interface is not intended to be implemented by clients.
*/ */
public interface ISymbolReader { public interface ISymbolReader extends AutoCloseable {
String[] getSourceFiles(); String[] getSourceFiles();
@ -35,4 +35,7 @@ public interface ISymbolReader {
* @since 5.2 * @since 5.2
*/ */
String[] getSourceFiles(IProgressMonitor monitor); String[] getSourceFiles(IProgressMonitor monitor);
@Override
void close();
} }

View file

@ -13,6 +13,7 @@
*******************************************************************************/ *******************************************************************************/
package org.eclipse.cdt.utils.coff; package org.eclipse.cdt.utils.coff;
import java.io.FileNotFoundException;
import java.io.IOException; import java.io.IOException;
import java.io.RandomAccessFile; import java.io.RandomAccessFile;
import java.util.ArrayList; import java.util.ArrayList;
@ -30,14 +31,44 @@ public class CodeViewReader implements ISymbolReader {
private String[] files = null; private String[] files = null;
private boolean parsed = false; private boolean parsed = false;
public CodeViewReader(RandomAccessFile accessFile, int dataOffset, boolean littleEndian) { /**
file = accessFile; * @since 8.0
*/
public CodeViewReader(String filename, int dataOffset, boolean littleEndian) throws FileNotFoundException {
file = new RandomAccessFile(filename, "r"); //$NON-NLS-1$
cvData = dataOffset; cvData = dataOffset;
isLe = littleEndian; isLe = littleEndian;
fileList = new ArrayList<>(); fileList = new ArrayList<>();
} }
@Override
public void close() {
dispose();
}
/**
* @since 8.0
*/
public void dispose() {
if (file != null) {
try {
file.close();
} catch (IOException e) {
}
file = null;
}
}
@Override
protected void finalize() throws Throwable {
try {
dispose();
} finally {
super.finalize();
}
}
@Override @Override
public String[] getSourceFiles() { public String[] getSourceFiles() {
if (!parsed) { if (!parsed) {

View file

@ -579,13 +579,16 @@ public class PE implements AutoCloseable {
} }
@Override @Override
public void close() throws IOException { public void close() {
dispose(); dispose();
} }
public void dispose() throws IOException { public void dispose() {
if (rfile != null) { if (rfile != null) {
try {
rfile.close(); rfile.close();
} catch (IOException e) {
}
rfile = null; rfile = null;
} }
} }
@ -771,7 +774,6 @@ public class PE implements AutoCloseable {
} }
private ISymbolReader createCodeViewReader() { private ISymbolReader createCodeViewReader() {
ISymbolReader symReader = null;
final int IMAGE_DIRECTORY_ENTRY_DEBUG = 6; final int IMAGE_DIRECTORY_ENTRY_DEBUG = 6;
try { try {
@ -813,8 +815,7 @@ public class PE implements AutoCloseable {
String s2 = accessFile.readLine(); String s2 = accessFile.readLine();
if (s2.startsWith("NB11")) { //$NON-NLS-1$ if (s2.startsWith("NB11")) { //$NON-NLS-1$
Attribute att = getAttribute(); Attribute att = getAttribute();
symReader = new CodeViewReader(accessFile, debugBase, att.isLittleEndian()); return new CodeViewReader(filename, debugBase, att.isLittleEndian());
return symReader;
} }
} }
fileOffset += dir.DEBUGDIRSZ; fileOffset += dir.DEBUGDIRSZ;
@ -825,7 +826,7 @@ public class PE implements AutoCloseable {
e.printStackTrace(); e.printStackTrace();
} }
return symReader; return null;
} }
private ISymbolReader createStabsReader() { private ISymbolReader createStabsReader() {

View file

@ -673,13 +673,16 @@ public class PE64 implements AutoCloseable {
} }
@Override @Override
public void close() throws IOException { public void close() {
dispose(); dispose();
} }
public void dispose() throws IOException { public void dispose() {
if (rfile != null) { if (rfile != null) {
try {
rfile.close(); rfile.close();
} catch (IOException e) {
}
rfile = null; rfile = null;
} }
} }
@ -881,7 +884,6 @@ public class PE64 implements AutoCloseable {
} }
private ISymbolReader createCodeViewReader() { private ISymbolReader createCodeViewReader() {
ISymbolReader symReader = null;
final int IMAGE_DIRECTORY_ENTRY_DEBUG = 6; final int IMAGE_DIRECTORY_ENTRY_DEBUG = 6;
try { try {
@ -942,8 +944,7 @@ public class PE64 implements AutoCloseable {
String s2 = accessFile.readLine(); String s2 = accessFile.readLine();
if (s2.startsWith("NB11")) { //$NON-NLS-1$ if (s2.startsWith("NB11")) { //$NON-NLS-1$
Attribute att = getAttribute(); Attribute att = getAttribute();
symReader = new CodeViewReader(accessFile, debugBase, att.isLittleEndian()); return new CodeViewReader(filename, debugBase, att.isLittleEndian());
return symReader;
} }
} }
fileOffset += dir.DEBUGDIRSZ; fileOffset += dir.DEBUGDIRSZ;
@ -954,7 +955,7 @@ public class PE64 implements AutoCloseable {
e.printStackTrace(); e.printStackTrace();
} }
return symReader; return null;
} }
private ISymbolReader createStabsReader() { private ISymbolReader createStabsReader() {

View file

@ -107,11 +107,12 @@ public class PEBinaryObject extends BinaryObjectAdapter {
} }
} }
if (adapter.equals(ISymbolReader.class)) { if (adapter.equals(ISymbolReader.class)) {
PE pe = getAdapter(PE.class); try (PE pe = getAdapter(PE.class)) {
if (pe != null) { if (pe != null) {
return (T) pe.getSymbolReader(); return (T) pe.getSymbolReader();
} }
} }
}
return super.getAdapter(adapter); return super.getAdapter(adapter);
} }

View file

@ -109,11 +109,12 @@ public class PEBinaryObject64 extends BinaryObjectAdapter {
} }
} }
if (adapter.equals(ISymbolReader.class)) { if (adapter.equals(ISymbolReader.class)) {
PE64 pe = getAdapter(PE64.class); try (PE64 pe = getAdapter(PE64.class)) {
if (pe != null) { if (pe != null) {
return (T) pe.getSymbolReader(); return (T) pe.getSymbolReader();
} }
} }
}
return super.getAdapter(adapter); return super.getAdapter(adapter);
} }

View file

@ -39,7 +39,7 @@ import org.eclipse.cdt.utils.elf.Elf.Section;
import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.Path; import org.eclipse.core.runtime.Path;
public class Dwarf { public class Dwarf implements AutoCloseable {
/* Section names. */ /* Section names. */
final static String DWARF_DEBUG_INFO = ".debug_info"; //$NON-NLS-1$ final static String DWARF_DEBUG_INFO = ".debug_info"; //$NON-NLS-1$
@ -359,6 +359,26 @@ public class Dwarf {
} }
private void dispose() {
dwarfSections.clear();
dwarfAltSections.clear();
System.gc();
}
@Override
public void close() {
dispose();
}
@Override
protected void finalize() throws Throwable {
try {
dispose();
} finally {
super.finalize();
}
}
int read_4_bytes(ByteBuffer in) throws IOException { int read_4_bytes(ByteBuffer in) throws IOException {
try { try {
byte[] bytes = new byte[4]; byte[] bytes = new byte[4];

View file

@ -248,4 +248,7 @@ public class StabsReader implements ISymbolReader {
return getSourceFiles(); return getSourceFiles();
} }
@Override
public void close() {
}
} }

View file

@ -59,7 +59,6 @@ public class Elf implements AutoCloseable {
private Symbol[] symbolsTable; private Symbol[] symbolsTable;
/** .dynSym section */ /** .dynSym section */
private Symbol[] dynamicSymbols; private Symbol[] dynamicSymbols;
private boolean areSectionsMapped; // Have sections been mapped? Used to clean up properly in Elf.Dispose.
protected String EMPTY_STRING = ""; //$NON-NLS-1$ protected String EMPTY_STRING = ""; //$NON-NLS-1$
private long elfOffset; private long elfOffset;
@ -339,7 +338,6 @@ public class Elf implements AutoCloseable {
*/ */
public ByteBuffer mapSectionData() throws IOException { public ByteBuffer mapSectionData() throws IOException {
makeSureNotCompressed(); makeSureNotCompressed();
areSectionsMapped = true;
return efile.getChannel().map(MapMode.READ_ONLY, sh_offset, sh_size).load().asReadOnlyBuffer(); return efile.getChannel().map(MapMode.READ_ONLY, sh_offset, sh_size).load().asReadOnlyBuffer();
} }
@ -987,9 +985,6 @@ public class Elf implements AutoCloseable {
if (efile != null) { if (efile != null) {
efile.close(); efile.close();
efile = null; efile = null;
// ensure the mappings get cleaned up
if (areSectionsMapped)
System.gc();
} }
} catch (IOException e) { } catch (IOException e) {
} }
@ -1304,6 +1299,12 @@ public class Elf implements AutoCloseable {
return reader; return reader;
} }
/**
* Creates a new symbol reader instance on each call. Caller is responsible for closing
* the symbol reader
*
* @return symbol reader or {@code null} if couldn't create symbol reader
*/
public ISymbolReader getSymbolReader() { public ISymbolReader getSymbolReader() {
ISymbolReader reader = null; ISymbolReader reader = null;
reader = createDwarfReader(); reader = createDwarfReader();

View file

@ -187,11 +187,12 @@ public class ElfBinaryObject extends BinaryObjectAdapter {
} }
} }
if (adapter.equals(ISymbolReader.class)) { if (adapter.equals(ISymbolReader.class)) {
Elf elf = getAdapter(Elf.class); try (Elf elf = getAdapter(Elf.class)) {
if (elf != null) { if (elf != null) {
return (T) elf.getSymbolReader(); return (T) elf.getSymbolReader();
} }
} }
}
return super.getAdapter(adapter); return super.getAdapter(adapter);
} }

View file

@ -34,7 +34,7 @@ import org.eclipse.cdt.utils.debug.stabs.StabsReader;
* This class is planned for removal in next major release. * This class is planned for removal in next major release.
*/ */
@Deprecated @Deprecated
public class MachO { public class MachO implements AutoCloseable {
protected ERandomAccessFile efile; protected ERandomAccessFile efile;
protected MachOhdr mhdr; protected MachOhdr mhdr;
@ -1109,6 +1109,11 @@ public class MachO {
} }
} }
@Override
public void close() {
dispose();
}
public void dispose() { public void dispose() {
if (cppFilt != null) { if (cppFilt != null) {
cppFilt.dispose(); cppFilt.dispose();

View file

@ -32,7 +32,7 @@ import org.eclipse.cdt.utils.debug.stabs.StabsReader;
/** /**
* @since 5.2 * @since 5.2
*/ */
public class MachO64 { public class MachO64 implements AutoCloseable {
protected ERandomAccessFile efile; protected ERandomAccessFile efile;
protected MachOhdr mhdr; protected MachOhdr mhdr;
@ -1184,6 +1184,11 @@ public class MachO64 {
} }
} }
@Override
public void close() {
dispose();
}
public void dispose() { public void dispose() {
if (cppFilt != null) { if (cppFilt != null) {
cppFilt.dispose(); cppFilt.dispose();

View file

@ -395,11 +395,12 @@ public class MachOBinaryObject extends BinaryObjectAdapter {
} }
} }
if (adapter.equals(ISymbolReader.class)) { if (adapter.equals(ISymbolReader.class)) {
MachO macho = getAdapter(MachO.class); try (MachO macho = getAdapter(MachO.class)) {
if (macho != null) { if (macho != null) {
return (T) macho.getSymbolReader(); return (T) macho.getSymbolReader();
} }
} }
}
return super.getAdapter(adapter); return super.getAdapter(adapter);
} }

View file

@ -410,11 +410,12 @@ public class MachOBinaryObject64 extends BinaryObjectAdapter {
} }
} }
if (adapter.equals(ISymbolReader.class)) { if (adapter.equals(ISymbolReader.class)) {
MachO64 macho = getAdapter(MachO64.class); try (MachO64 macho = getAdapter(MachO64.class)) {
if (macho != null) { if (macho != null) {
return (T) macho.getSymbolReader(); return (T) macho.getSymbolReader();
} }
} }
}
return super.getAdapter(adapter); return super.getAdapter(adapter);
} }

View file

@ -66,6 +66,7 @@ public class CompilerOptionParser implements IWorkspaceRunnable {
@Override @Override
public void run(IProgressMonitor monitor) { public void run(IProgressMonitor monitor) {
ISymbolReader reader = null;
try { try {
// Calculate how many source files we have to process and use that as a basis // Calculate how many source files we have to process and use that as a basis
// for our work estimate. // for our work estimate.
@ -102,7 +103,7 @@ public class CompilerOptionParser implements IWorkspaceRunnable {
return; return;
} }
ISymbolReader reader = bf.getAdapter(ISymbolReader.class); reader = bf.getAdapter(ISymbolReader.class);
String[] sourceFiles = reader.getSourceFiles(); String[] sourceFiles = reader.getSourceFiles();
monitor.beginTask(Messages.GetCompilerOptions, sourceFiles.length * 2 + 1); monitor.beginTask(Messages.GetCompilerOptions, sourceFiles.length * 2 + 1);
@ -157,6 +158,9 @@ public class CompilerOptionParser implements IWorkspaceRunnable {
e.printStackTrace(); e.printStackTrace();
} catch (IOException e1) { } catch (IOException e1) {
e1.printStackTrace(); e1.printStackTrace();
} finally {
if (reader != null)
reader.close();
} }
monitor.done(); monitor.done();
} }

View file

@ -120,11 +120,11 @@ public class StandardSourceFilesProvider extends PlatformObject implements ISour
IBinaryFile bin = createBinaryFile(executable); IBinaryFile bin = createBinaryFile(executable);
if (bin != null) { if (bin != null) {
ISymbolReader symbolreader = bin.getAdapter(ISymbolReader.class); try (ISymbolReader symbolreader = bin.getAdapter(ISymbolReader.class)) {
if (symbolreader != null) { if (symbolreader != null) {
return symbolreader.getSourceFiles(monitor); return symbolreader.getSourceFiles(monitor);
} }
}
} }
return new String[0]; return new String[0];
} }