Leaking this constructor

Question

Possible Duplicate:
“this” reference escaping during construction?

I would like to know whether leaking this in constructor issue is applied on this instance, since i believe the constructor has to invoke setJMenuBar() method to finish its construction, here is class implementation:

public class StaffManagerMainWindow extends JFrame implements ActionListener {

    public StaffManagerMainWindow(String title, Image icon) throws HeadlessException {
        ...
        setJMenuBar();
    }

    private void setJMenuBar() {
        ...
        exitItem.addActionListener(this);
        ...
    }

    @Override
    public void actionPerformed(ActionEvent e) {
        throw new UnsupportedOperationException("Not supported yet.");
    }
}

If i move all code from setJMenuBar() method inside constructor then i get warning message "leaking this in constructor" but for my case the compiler doesn't complain anything — MinhHoang, Apr 25 '12 at 15:26
This just means that the static analysis performed on the code is limited -- it just checks whether you are passing `this` explicitly in a method call. — Marko Topolnik, Apr 25 '12 at 15:33
@FinalIllusion There is only so much the compiler can work out. It can't feasibly work out warnings in all situations. — Jim, Apr 25 '12 at 15:33

score 3 · Accepted Answer · answered Apr 25 '12 at 15:23

3

I think there is a leak in the constructor. You are passing "this" before the complete initiallization of the object.

answered Apr 25 '12 at 15:23

Edge

722
2
7
21

so why the compiler didn't give any warning message ? – MinhHoang Apr 25 '12 at 15:29
1

I have never heard of the compiler checking for a leaked `this`. It is not a problem on its own, it just opens the door for problems. Are you positive this is a compiler warning, and not some other checking tool you have? What compiler are you using? Java/Eclipse, which version? Just curious. – Marko Topolnik Apr 25 '12 at 15:30
@MarkoTopolnik: Netbeans IDE sir – MinhHoang Apr 25 '12 at 15:34
Thanks. Netbeans relies on `javac` for compilation. In Eclipse Helios this code in constructor causes no warning: `new JButton().addActionListener(this);` Compiled that with Java 6 `javac`, also no warning. What Java version are you using? – Marko Topolnik Apr 25 '12 at 15:45

Kjetil · Answer 2 · 2012-04-25T16:19:33.513

1

Leaking this in the constructor can be an issue if your program is multithreaded. You're giving this to exitItem, which may invoke actionPerformed() from another thread before your constructor finishes. This is not OK. It can break your program really really badly, especially if you add subclasses to the picture.

Create an init() method and a factory method instead:

public class StaffManager {
    public static StaffManager create() {
        StaffManager staffManager = new StaffManager();
        staffManager.init();
        return staffManager;
    }

    private StaffManager() {
    }

    private void init() {
        // Add listeners here.
    }
}

Edit: Because Swing objects are always created on the event-dispatch thread, leaking this will not have any effect in this case. Nevertheless I prefer not doing it.

edited Apr 25 '12 at 16:19

answered Apr 25 '12 at 15:38

Kjetil

266
2
3

C'mon now, Kjetil, why would any legal code call a Swing callback outside the Event-Dispatch Thread? Even if it did, that would be a problem in itself. – Marko Topolnik Apr 25 '12 at 15:41
I don't know the entire picture here, but I'm guessing that this class may be initialized in a different thread at the same time as an event occurs. In any case leaking this is a bad idea, particularly if the class is meant to be thread safe. – Kjetil Apr 25 '12 at 15:48
Swing UI classes are by definition not thread-safe -- all Swing code **MUST** be executed on the event-dispatch thread. All event listeners will definitely be executed exclusively on the EDT. – Marko Topolnik Apr 25 '12 at 15:51
Do you have to construct all Swing objects on the event-dispatch thread? If not, the issue applies here. – Kjetil Apr 25 '12 at 15:54
yes all Swing components should be executed on EDT thank you for answering my question – MinhHoang Apr 25 '12 at 15:59
According to http://java.sun.com/products/jfc/tsc/articles/threads/threads1.html#single_thread_rule it is OK to construct swing objects in any thread. Be careful ;) – Kjetil Apr 25 '12 at 16:06
But even that is quite irrelevant in this context -- it is not good design to go fending off invisible enemies, throwing in unnecessary complexity, when clearly this is not going to cause any problems. There are myriads of problems waiting for any Swing programmer, the last he needs is taking care of virtual ones. – Marko Topolnik Apr 25 '12 at 16:09
@Kjetil that article has almost the same age as my granny. The new Swing rule is indeed to even create all objects on the EDT. – Robin Apr 25 '12 at 16:13

Leaking this constructor

2 Answers2