11

I am confounded with a strange issue. Basically the situation is like this. I implemented the runnable in my class, I pass the class in a new thread, I override my run() method within the class that implements runnable and then I start the thread. However, my start() method never calls my run() method. I've search the forums but I can't seem to find another similar problem.

Below is my sample code:

public class EmailManager implements Runnable {
    PortalManagementSBLocal pmbr= this.lookupPortalManagementSB();
    Thread runner;
    String emailServerName = "";
    String smtpPort = "";
    String emailTo = "";
    String emailFrom = "";
    String mailer = "JavaMailer";
    String subject = "";
    String message = "";

    public EmailManager() {
    }//default constructor

    public EmailManager(String emailTo, String subject, String message){

        this.emailTo=emailTo;
        this.subject = subject;
        this.message = message;
        //need to make this dynamic
        this.emailFrom = pmbr.getEmailFrom();
        this.emailServerName = pmbr.getEmailServerName();
        this.smtpPort = pmbr.getEmailSMTPPort();
        //runner = new Thread(this,"Email");
        runner = new Thread(this);
        runner.start();
        System.out.println("***** Email Thread running...");


    }

    @Override
    public void run(){
        sendEmail(); //This is never called
    }

Would really appreciate any guidance! Thanks a ton!

owe
  • 4,890
  • 7
  • 36
  • 47
Qin Zhengquan
  • 467
  • 1
  • 8
  • 20
  • And you are sure start() is being called? Can you add a log message to the start of run(), before sendMail() is called? – Peter Lawrey Jun 20 '12 at 12:09
  • Do you constructing the derived type of EmailManager? – Prince John Wesley Jun 20 '12 at 12:09
  • 1
    Nasty design, instantiating the thread and starting it in the constructor. Almost certain to generate race conditions. – Qwerky Jun 20 '12 at 12:20
  • @Qwerky I'm not saying this is a good design, but I don't see how it would generate race conditions. This is no different, execution-wise, from starting a thread outside the constructor each time an object of this type is instantiated. – Erick Robertson Jun 21 '12 at 11:54

5 Answers5

8

How do you know that this method is never called?

The simple test below works. So there's no problem creating a thread and running it from within the constructor. So there's something else going on that's preventing you from seeing that sendEmail() is being called.

public class Test implements Runnable {
  Thread runner;
  public Test() {
    this.runner = new Thread(this);
    this.runner.start();
  }

  @Override
  public void run() {
    System.out.println("ya");   
  }

  public static void main(String[] args) {
    new Test();
  }
}
Erick Robertson
  • 32,125
  • 13
  • 69
  • 98
  • 3
    Hi all thanks for the post! I realize that my debugger was running the main thread instead of the runner Thread. Also there was a problem with my sendEmail method, so I initially thought the run method was not executed. Thanks for the guidance once again! – Qin Zhengquan Jun 21 '12 at 06:41
  • 2
    "debugger was running the main thread", that's what my problem was! Cheers user 1433483 – Felix Jun 20 '13 at 02:56
3

i think, the problem is that you're passing this before the constructor call is complete. This might help you: https://stackoverflow.com/a/5623327/1441485

Community
  • 1
  • 1
iozee
  • 1,339
  • 1
  • 12
  • 24
1

dont use runner = new Thread(this); in a constructor

move "runner = new Thread(this); runner.start();

to init function, create instance using new and call this init()

Subin Sebastian
  • 10,870
  • 3
  • 37
  • 42
1
runner = new Thread(this);
        runner.start();

Your this object is not properly initialized, until the constructor returns. So move it our somewhere, may be to where your spawning this new thread.

Ahmad
  • 2,110
  • 5
  • 26
  • 36
  • To clarify your comment, I found this answer interesting : http://stackoverflow.com/a/5623396/3519951 A partially initialized object is problematic only if it has uninitialized fields. In this question's case, launching the thread is the last thing done in the constructor, therefore the "partially initialized" state is harmless. – JM Lord Sep 18 '15 at 18:19
-3

Move it out of the constructor. There is no "this" so to speak inside the constructor.

ChadNC
  • 2,528
  • 4
  • 25
  • 39