Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 314504

Summary: Attach to process launch leaks file descriptors
Product: [Tools] CDT Reporter: James Blackburn <jamesblackburn+eclipse>
Component: cdt-debugAssignee: James Blackburn <jamesblackburn+eclipse>
Status: RESOLVED FIXED QA Contact: Ken Ryall <ken.ryall>
Severity: normal    
Priority: P3 CC: marc.khouzam, pawel.1.piech
Version: 7.0Flags: marc.khouzam: review+
Target Milestone: ---   
Hardware: PC   
OS: All   
Whiteboard:
Attachments:
Description Flags
file descriptor leak
none
proposed patch
jamesblackburn+eclipse: iplog-
patch 2 jamesblackburn+eclipse: iplog-

Description James Blackburn CLA 2010-05-26 12:32:26 EDT 
Created attachment 170038 [details]
file descriptor leak

It looks like the attack to process launch is leaking file descriptors which can make Eclipse unusable.

The attachment shows ls -l of the /proc/<java_pid>/fd. It shows filedescriptors still open to elsewhere in the proc tree. 

I had previously used the DSF attach to process launcher, so I'm guessing it's this that's looking at cmdline.
Comment 1 Marc Khouzam CLA 2010-05-26 14:28:00 EDT 
What are the steps to do this?
When I try an DSF-GDB attach on a Linux 32, I just get

> ls -l /proc/10933/fd/
total 3
lrwx------ 1 lmckhou lmc_x 64 2010-05-26 14:26 0 -> /dev/pts/1
lrwx------ 1 lmckhou lmc_x 64 2010-05-26 14:26 1 -> /dev/pts/1
lrwx------ 1 lmckhou lmc_x 64 2010-05-26 14:26 2 -> /dev/pts/1
Comment 2 James Blackburn CLA 2010-05-26 14:45:05 EDT 
I get this just starting an attach launch.

These issues tend to be very JVM dependent. While file descriptors are closed during garbage collection, as garbage collection is triggered by memory pressure the clean-up can be infinitely delayed. I'm guessing whatever in the attach dialog opens cmdline doesn't explicitly close the input stream. 

I can dig a bit deeper; the fix is usually trivial.
Comment 3 James Blackburn CLA 2010-05-26 15:29:18 EDT 
Created attachment 170078 [details]
proposed patch

ProcessList never closes the FileReader which leaks file descriptors. 
Simple fix attached, will give it a go tomorrow when I'm back on Linux.
Comment 4 Marc Khouzam CLA 2010-05-26 21:06:49 EDT 
Wow, that's a old bug.
There is a similar class in the win32 plugin which creates an InputStreamReader.  Should that one also call close()?
Comment 5 James Blackburn CLA 2010-05-27 05:59:18 EDT 
Created attachment 170158 [details]
patch 2 

(In reply to comment #4)
> Wow, that's a old bug.
> There is a similar class in the win32 plugin which creates an
> InputStreamReader.  Should that one also call close()?

Yes, they all should.  It's much less bad here though, as we're only leaking one process worth of filedescriptors (per query), rather than one file descriptor for every process on the system.

I did some tracing through DSF (so many layers!) and it looks like it only falls back to these core methods when GDB doesn't list the processes (GDBProcesses_7_0#getRunningProcesses).  So this issue is most observable using CDI.

I've been through all the ProcessLists and ensure that filedescriptors and close()d and processes destroy()d.  I think it's pretty straightforward, but do review and shout if you see anything untoward!
Comment 6 James Blackburn CLA 2010-05-27 06:03:00 EDT 
Committed. Marc if you could give it a look over, that would be appreciated.
Comment 8 Marc Khouzam CLA 2010-05-27 09:21:32 EDT 
Looks good