Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.java.programmer > #12561 > unrolled thread

Strange Socket problem

Back to article view | Back to comp.lang.java.programmer

Contents

  Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-01 11:49 -0800
    Re: Strange Socket problem Steven Simpson <ss@domain.invalid> - 2012-03-01 20:59 +0000
      Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-01 13:30 -0800
        Re: Strange Socket problem Steven Simpson <ss@domain.invalid> - 2012-03-01 22:54 +0000
          Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-01 16:32 -0800
            Re: Strange Socket problem Steven Simpson <ss@domain.invalid> - 2012-03-03 09:08 +0000
              Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-03 17:48 -0800
    Re: Strange Socket problem markspace <-@.> - 2012-03-01 14:08 -0800
      Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-01 16:48 -0800
        Re: Strange Socket problem markspace <-@.> - 2012-03-02 08:33 -0800
          Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-02 15:29 -0800
            Re: Strange Socket problem markspace <-@.> - 2012-03-03 09:33 -0800
              Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-03 17:55 -0800
                Re: Strange Socket problem markspace <-@.> - 2012-03-03 20:06 -0800
                  Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-03 21:02 -0800
                    Re: Strange Socket problem "John B. Matthews" <nospam@nospam.invalid> - 2012-03-05 00:00 -0500
                      Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-05 09:44 -0800
    Re: Strange Socket problem Martin Gregorie <martin@address-in-sig.invalid> - 2012-03-01 22:15 +0000
      Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-01 16:43 -0800
        Re: Strange Socket problem Martin Gregorie <martin@address-in-sig.invalid> - 2012-03-03 02:01 +0000
    Reiteration: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-01 16:51 -0800
      Re: Reiteration: Strange Socket problem Martin Gregorie <martin@address-in-sig.invalid> - 2012-03-03 02:02 +0000
    Re: Strange Socket problem x <x@x.x> - 2012-03-02 21:18 +0100
      Re: Strange Socket problem Lew <noone@lewscanon.com> - 2012-03-02 15:06 -0800
        Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-02 15:38 -0800
          Re: Strange Socket problem Lew <noone@lewscanon.com> - 2012-03-02 16:00 -0800
            Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-02 17:18 -0800
              Re: Strange Socket problem Martin Gregorie <martin@address-in-sig.invalid> - 2012-03-03 02:13 +0000
                Re: Strange Socket problem Steven Simpson <ss@domain.invalid> - 2012-03-03 08:41 +0000
                  Re: Strange Socket problem Martin Gregorie <martin@address-in-sig.invalid> - 2012-03-03 18:26 +0000
          Re: Strange Socket problem x <x@x.x> - 2012-03-03 11:32 +0100
            Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-03 17:59 -0800
            Re: Strange Socket problem Lew <noone@lewscanon.com> - 2012-03-03 23:21 -0800
        Re: Strange Socket problem x <x@x.x> - 2012-03-03 10:59 +0100
          Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-03 18:08 -0800
      Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-02 15:33 -0800
    Re: Strange Socket problem Paka Small <paka-en@tumia.org> - 2012-03-03 03:32 -0800
      Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-03 18:00 -0800
    Re: Strange Socket problem x <x@x.x> - 2012-03-03 12:45 +0100
      Re: Strange Socket problem Knute Johnson <nospam@knutejohnson.com> - 2012-03-03 18:02 -0800

Page 2 of 2 — ← Prev page 1 [2]

#12573 — Reiteration: Strange Socket problem

All three systems run fine until they hang.  There are normal timeouts 
early in the morning when there is no data available and they all 
reconnect without a hitch.

I'm pretty sure the fault is in the Socket constructor somewhere.  It 
happens at all three sites at the same time so I think it is triggered 
by something the server does.

Anybody think it could be caused by the keep alive?

-- 

Knute Johnson

[toc] | [prev] | [next] | [standalone]

#12615 — Re: Reiteration: Strange Socket problem

On Thu, 01 Mar 2012 16:51:54 -0800, Knute Johnson wrote:

> All three systems run fine until they hang.  There are normal timeouts
> early in the morning when there is no data available and they all
> reconnect without a hitch.
> 
> I'm pretty sure the fault is in the Socket constructor somewhere.  It
> happens at all three sites at the same time so I think it is triggered
> by something the server does.
> 
> Anybody think it could be caused by the keep alive?
>
I've never used it, so won't speculate.


-- 
martin@   | Martin Gregorie
gregorie. | Essex, UK
org       |

[toc] | [prev] | [next] | [standalone]

#12591

Knute Johnson pisze:
> I'm having a problem in some production code that I can't figure out. 

Can you please describe the usage of your class ?

I'm rather puzzled about having a Thread object inside a Runnable. Your 
  class makes it is extremely easy to have TWO threads accessing private 
fields, this is rarely a good idea. (yes, I have recreated it :-)

What exactly is the point of the public start() method? I must say it 
looks suspicious to me.

Best regards,
Marcin Jaskolski

[toc] | [prev] | [next] | [standalone]

#12602

On 03/02/2012 12:18 PM, x wrote:
> Knute Johnson pisze:
>> I'm having a problem in some production code that I can't figure out.
>
> Can you please describe the usage of your class ?
>
> I'm rather puzzled about having a Thread object inside a Runnable. Your class

The issue would be what's used inside the 'run()' method, not just what fields 
the class has.

I agree that the 'thread' variable's scope is wrong - it should be local to 
'start()', not an instance member. But at the moment that's not causing any 
outright harm.

> makes it is extremely easy to have TWO threads accessing private fields, this
> is rarely a good idea. (yes, I have recreated it :-)

His class spawns one thread per instance. The main thread doesn't use the 
mutable fields. The only common access of note is the 'runFlag' mechanism, 
which is bog-standard in his code. The 'thread' member is not referenced 
within 'run()'. I'm not sure why all his other instance variables are 
'volatile', but at first blush I don't see a problem with his synchronization 
safety.

You need to be specific about the trouble you claim you found. Show us where, 
because I don't see danger coming from the areas you cited.

> What exactly is the point of the public start() method? I must say it looks
> suspicious to me.

It's also bog standard, and bog simple. Its purpose is to start the socket thread.

  public class Client
  {
    public static void main(String[] args)
    {
      new SportsWinClient().start();
    }
  }

-- 
Lew
Honi soit qui mal y pense.
http://upload.wikimedia.org/wikipedia/commons/c/cf/Friz.jpg

[toc] | [prev] | [next] | [standalone]

#12605

On 3/2/2012 3:06 PM, Lew wrote:
> I agree that the 'thread' variable's scope is wrong - it should be local
> to 'start()', not an instance member. But at the moment that's not
> causing any outright harm.

It's so I can start and interrupt the thread from a method call.

The volatiles exist because the methods that access them can be called 
from other threads.  I could have synchronized the start() stop() 
methods but not easily the socket variable in the run() method.  I 
thought it was cleaner to just use volatile.

-- 

Knute Johnson

[toc] | [prev] | [next] | [standalone]

#12607

Knute Johnson wrote:
> The volatiles exist because the methods that access them can be called from
> other threads. I could have synchronized the start() stop() methods but not
> easily the socket variable in the run() method. I thought it was cleaner to
> just use volatile.

I see a problem right there.

>     public void disconnect() {
>         if (isConnected())
>             if (socket != null)
>                 try {
>                     socket.close();
>                 } catch (IOException ioe) {
>                     ioe.printStackTrace();
>                 }
>     }

Since these are controlled by separate synchronization (different 'volatile' 
variables) there's a race condition trying to work with both at once.

Also, 'socket' can become 'null' between the check for not 'null' and the 
'close()' call.

You need to synchronize with 'synchronized' or other strong mechanism.

-- 
Lew
Honi soit qui mal y pense.
http://upload.wikimedia.org/wikipedia/commons/c/cf/Friz.jpg

[toc] | [prev] | [next] | [standalone]

#12611

On 3/2/2012 4:00 PM, Lew wrote:
> Knute Johnson wrote:
>> The volatiles exist because the methods that access them can be called
>> from
>> other threads. I could have synchronized the start() stop() methods
>> but not
>> easily the socket variable in the run() method. I thought it was
>> cleaner to
>> just use volatile.
>
> I see a problem right there.
>
>> public void disconnect() {
>> if (isConnected())
>> if (socket != null)
>> try {
>> socket.close();
>> } catch (IOException ioe) {
>> ioe.printStackTrace();
>> }
>> }
>
> Since these are controlled by separate synchronization (different
> 'volatile' variables) there's a race condition trying to work with both
> at once.

I don't think so.

> Also, 'socket' can become 'null' between the check for not 'null' and
> the 'close()' call.

Only if I set it to null and I don't.  That is there so that if the 
Socket doesn't make the first connection when disconnect() is called 
that I won't get a NPE.

> You need to synchronize with 'synchronized' or other strong mechanism.

I don't think so and here's why; isConnected only gets modified by one 
thread and read by another, socket is only modified by one thread and 
read by another.  In the disconnect() method, as soon as isConnected() 
is called, isConnected the volatile variable is read and that would make 
socket current even if it weren't volatile which it is but I didn't want 
to rely on side effects in case I changed code somewhere.

Anyway, disconnect() isn't getting called in this situation so it's not 
causing my problem.

I really appreciate everybody looking at this.  I've got a couple of 
ideas of where to code some traps and I'll have to put those in one 
night and see what happens.

-- 

Knute Johnson

[toc] | [prev] | [next] | [standalone]

#12616

On Fri, 02 Mar 2012 17:18:04 -0800, Knute Johnson wrote:

> On 3/2/2012 4:00 PM, Lew wrote:
>> Knute Johnson wrote:
>>> The volatiles exist because the methods that access them can be called
>>> from other threads. I could have synchronized the start() stop()
>>> methods but not easily the socket variable in the run() method. I
>>> thought it was cleaner to just use volatile.
>>
>> I see a problem right there.
>>
>>> public void disconnect() {
>>> if (isConnected())
>>> if (socket != null)
>>> try {
>>> socket.close();
>>> } catch (IOException ioe) {
>>> ioe.printStackTrace();
>>> }
>>> }
>>
>> Since these are controlled by separate synchronization (different
>> 'volatile' variables) there's a race condition trying to work with both
>> at once.
> 
> I don't think so.
> 
>> Also, 'socket' can become 'null' between the check for not 'null' and
>> the 'close()' call.
> 
> Only if I set it to null and I don't.  That is there so that if the
> Socket doesn't make the first connection when disconnect() is called
> that I won't get a NPE.
> 
>> You need to synchronize with 'synchronized' or other strong mechanism.
> 
> I don't think so and here's why; isConnected only gets modified by one
> thread and read by another, socket is only modified by one thread and
> read by another.  In the disconnect() method, as soon as isConnected()
> is called, isConnected the volatile variable is read and that would make
> socket current even if it weren't volatile which it is but I didn't want
> to rely on side effects in case I changed code somewhere.
> 
> Anyway, disconnect() isn't getting called in this situation so it's not
> causing my problem.
> 
> I really appreciate everybody looking at this.  I've got a couple of
> ideas of where to code some traps and I'll have to put those in one
> night and see what happens.

new Socket() can return null (it says, but not why) but I don't think 
that is happening or you'd have seen an NPE.
 

-- 
martin@   | Martin Gregorie
gregorie. | Essex, UK
org       |

[toc] | [prev] | [next] | [standalone]

#12621

On 03/03/12 02:13, Martin Gregorie wrote:
> new Socket() can return null (it says, but not why) but I don't think
> that is happening or you'd have seen an NPE.

Where are you seeing this?  How can a constructor 'return' null?

<http://docs.oracle.com/javase/7/docs/api/java/net/Socket.html>

In another post, you also said (about InetAddress.getByName()):
> All I wanted to do was to flag up that its odd that the Javadocs say that
> sometimes it returns null without (apparently) throwing an exception but
> don't say explicitly what governs each behaviour, and that it would be
> useful to call it in such a way that that you force it to make that
> distinction explicit.

Again, I don't see where this is said.

<http://docs.oracle.com/javase/7/docs/api/java/net/InetAddress.html#getByName%28java.lang.String%29>

Am I looking at the wrong docs?

Cheers!

-- 
ss at comp dot lancs dot ac dot uk

[toc] | [prev] | [next] | [standalone]

#12631

On Sat, 03 Mar 2012 08:41:51 +0000, Steven Simpson wrote:

> Again, I don't see where this is said.
> 
> <http://docs.oracle.com/javase/7/docs/api/java/net/
InetAddress.html#getByName%28java.lang.String%29>
>
>
Nor do I now. I knew I was tired but didn't think I was tired enough to 
be making silly mistakes. Apologies. 


-- 
martin@   | Martin Gregorie
gregorie. | Essex, UK
org       |

[toc] | [prev] | [next] | [standalone]

#12624

Knute Johnson pisze:
> On 3/2/2012 3:06 PM, Lew wrote:
>> I agree that the 'thread' variable's scope is wrong - it should be local
>> to 'start()', not an instance member. But at the moment that's not
>> causing any outright harm.
> 
> It's so I can start and interrupt the thread from a method call.
> 
> The volatiles exist because the methods that access them can be called 
> from other threads.  I could have synchronized the start() stop() 

Other threads... What are other threads allowed to do ?
You see, I'm really troubled by the public methods:

public void disconnect()
public void stop()
public boolean isConnected()


> methods but not easily the socket variable in the run() method.  I 
> thought it was cleaner to just use volatile.

But, the volatile keyword does not provide any kind of access control. 
It just tells the JVM "hey, if this variable is changed by thread A, the 
changed value must be visible for thread B, so don't cache it too 
aggressively'.

Marcin Jaskolski

[toc] | [prev] | [next] | [standalone]

#12636

On 3/3/2012 2:32 AM, x wrote:
> Knute Johnson pisze:
>> On 3/2/2012 3:06 PM, Lew wrote:
>>> I agree that the 'thread' variable's scope is wrong - it should be local
>>> to 'start()', not an instance member. But at the moment that's not
>>> causing any outright harm.
>>
>> It's so I can start and interrupt the thread from a method call.
>>
>> The volatiles exist because the methods that access them can be called
>> from other threads. I could have synchronized the start() stop()
>
> Other threads... What are other threads allowed to do ?
> You see, I'm really troubled by the public methods:
>
> public void disconnect()
> public void stop()
> public boolean isConnected()

disconnect() is called by the main class to force a reload of the data
stop() is called my the main class exit routines
isConnected() is only called by disconnect() at the moment and will 
probably disappear in the next minor rewrite

>> methods but not easily the socket variable in the run() method. I
>> thought it was cleaner to just use volatile.
>
> But, the volatile keyword does not provide any kind of access control.
> It just tells the JVM "hey, if this variable is changed by thread A, the
> changed value must be visible for thread B, so don't cache it too
> aggressively'.
>
> Marcin Jaskolski

I know and I'm only using it for visibility in the methods when called 
by another class.

-- 

Knute Johnson

[toc] | [prev] | [next] | [standalone]

#12654

x wrote:
> But, the volatile keyword does not provide any kind of access control. It just
> tells the JVM "hey, if this variable is changed by thread A, the changed value
> must be visible for thread B, so don't cache it too aggressively'.

It does more than that.

All writes from thread A prior to the write to the volatile variable are 
visible from thread B after the latter reads that variable.

However I agree that multiple views of the same volatile variable introduce 
danger, as in the idiom

  if (volaVar != null)
  {
    volarVar.doSomething();
  }

because there's a threat window between the conditional check for non-'null' 
and the dereference to the method call where another thread could set the 
variable to a different value.

There's also a threat when you use two volatile variables to control a code block:

  if (volaVar1.equals(SOME_VALUE) && volaVar2.equals(DIFFERENT_VALUE)) ...

because 'volaVar1' can change by the time you test the second clause.

-- 
Lew
Honi soit qui mal y pense.
http://upload.wikimedia.org/wikipedia/commons/c/cf/Friz.jpg

[toc] | [prev] | [next] | [standalone]

#12623

Lew pisze:
> On 03/02/2012 12:18 PM, x wrote:
>> Knute Johnson pisze:
>>> I'm having a problem in some production code that I can't figure out.
>>
>> Can you please describe the usage of your class ?
>>
>> I'm rather puzzled about having a Thread object inside a Runnable. 
>> Your class
> 
> The issue would be what's used inside the 'run()' method, not just what 
> fields the class has.
> 
> I agree that the 'thread' variable's scope is wrong - it should be local 
> to 'start()', not an instance member. But at the moment that's not 
> causing any outright harm.
> 
>> makes it is extremely easy to have TWO threads accessing private 
>> fields, this
>> is rarely a good idea. (yes, I have recreated it :-)
> 
> His class spawns one thread per instance. The main thread doesn't use 
> the mutable fields. The only common access of note is the 'runFlag' 
> mechanism, which is bog-standard in his code. The 'thread' member is not 
> referenced within 'run()'. I'm not sure why all his other instance 
> variables are 'volatile', but at first blush I don't see a problem with 
> his synchronization safety.
> 
> You need to be specific about the trouble you claim you found. Show us 
> where, because I don't see danger coming from the areas you cited.

Consider the following scenario:

SportsWinClient swc = new SportsWinClient();
swc.start();
new Thread(swc).start();

(this exact code is unlikely to be present, but if the swc variable is 
passed around, someone, in a code far far away, may start the swc in a 
new thread. Simply because it's a Runnable.)

That is why I'm asking about the usage scenario.

>> What exactly is the point of the public start() method? I must say it 
>> looks
>> suspicious to me.
> 
> It's also bog standard, and bog simple. Its purpose is to start the 
> socket thread.

Is it standard... well, I'm not sure. As far as I know, the most typical 
thing to do would be to create a Runnable object (containing ONLY the 
task logic), and a separate Thread object:

Runnable myRunnable = new MyRunnable(...);
Threat t = new Thread(myRunnable);
t.start();



> 
>  public class Client
>  {
>    public static void main(String[] args)
>    {
>      new SportsWinClient().start();
>    }
>  }
> 

[toc] | [prev] | [next] | [standalone]

#12640

On 3/3/2012 1:59 AM, x wrote:
> Lew pisze:
>> On 03/02/2012 12:18 PM, x wrote:
>>> Knute Johnson pisze:
>>>> I'm having a problem in some production code that I can't figure out.
>>>
>>> Can you please describe the usage of your class ?
>>>
>>> I'm rather puzzled about having a Thread object inside a Runnable.
>>> Your class
>>
>> The issue would be what's used inside the 'run()' method, not just
>> what fields the class has.
>>
>> I agree that the 'thread' variable's scope is wrong - it should be
>> local to 'start()', not an instance member. But at the moment that's
>> not causing any outright harm.
>>
>>> makes it is extremely easy to have TWO threads accessing private
>>> fields, this
>>> is rarely a good idea. (yes, I have recreated it :-)
>>
>> His class spawns one thread per instance. The main thread doesn't use
>> the mutable fields. The only common access of note is the 'runFlag'
>> mechanism, which is bog-standard in his code. The 'thread' member is
>> not referenced within 'run()'. I'm not sure why all his other instance
>> variables are 'volatile', but at first blush I don't see a problem
>> with his synchronization safety.
>>
>> You need to be specific about the trouble you claim you found. Show us
>> where, because I don't see danger coming from the areas you cited.
>
> Consider the following scenario:
>
> SportsWinClient swc = new SportsWinClient();
> swc.start();
> new Thread(swc).start();
>
> (this exact code is unlikely to be present, but if the swc variable is
> passed around, someone, in a code far far away, may start the swc in a
> new thread. Simply because it's a Runnable.)
>
> That is why I'm asking about the usage scenario.
>
>>> What exactly is the point of the public start() method? I must say it
>>> looks
>>> suspicious to me.
>>
>> It's also bog standard, and bog simple. Its purpose is to start the
>> socket thread.
>
> Is it standard... well, I'm not sure. As far as I know, the most typical
> thing to do would be to create a Runnable object (containing ONLY the
> task logic), and a separate Thread object:
>
> Runnable myRunnable = new MyRunnable(...);
> Threat t = new Thread(myRunnable);
> t.start();
>
>
>
>>
>> public class Client
>> {
>> public static void main(String[] args)
>> {
>> new SportsWinClient().start();
>> }
>> }
>>

I'm not sure what your argument is.  It is easy enough either way to 
create or start numerous examples.  I don't and I won't and your 
scenario wouldn't prevent that either.  And if the Thread is started 
more than once it throws and exception.

-- 

Knute Johnson

[toc] | [prev] | [next] | [standalone]

#12604

On 3/2/2012 12:18 PM, x wrote:
> Knute Johnson pisze:
>> I'm having a problem in some production code that I can't figure out.
>
> Can you please describe the usage of your class ?
>
> I'm rather puzzled about having a Thread object inside a Runnable. Your
> class makes it is extremely easy to have TWO threads accessing private
> fields, this is rarely a good idea. (yes, I have recreated it :-)
>
> What exactly is the point of the public start() method? I must say it
> looks suspicious to me.
>
> Best regards,
> Marcin Jaskolski

Another class instantiates this class and starts it.  It is never 
started again because it should not ever stop unless some runtime 
exception is thrown.

It's not a Thread object inside a Runnable, it is a reference to the 
Thread object.  The Runnable is just that.  If two of these got started, 
it still probably wouldn't hurt anything.

-- 

Knute Johnson

[toc] | [prev] | [next] | [standalone]

#12625

On 1 mrt, 20:49, Knute Johnson <nos...@knutejohnson.com> wrote:
> I'm having a problem in some production code that I can't figure out.
> I'll post the complete actual code below.  This code is running in three
> places and has the same problem in two of them at the same time.  The
> other I'm not sure, it may be that the personnel operating it are
> restarting the program and so don't complain.  This piece of code is a
> simple client that connects via a Socket to a server.  The server
> supplies some data and the client reads that data and files it away.  It
> is supposed to restart itself if there is a connection failure or fault
> for whatever reason.  The problem is that at some random point in time
> the Socket disconnects, the code logs the disconnect but never restarts.
>   It does print the "SportsWinClient Disconnected" message but never
> executes the "fireconnectionEvent()" method after creating a new Socket.
>   It doesn't print any Exception message.  I'm not sure how it gets out
> of the try block without printing the "End of Stream" message or an
> exception message.
>
> The crazy part is that all night long when there is no activity from the
> server it times out and restarts with no problems.
>
> I'm hoping that somebody will see a fault in my code that could cause
> the failure.  It is not a compile problem so I left the formatting as it is.
>
> Thanks for looking.
>
> package com.knutejohnson.xyzcasinos.translux;
>
> import java.io.*;
> import java.net.*;
> import java.util.*;
>
> import com.knutejohnson.classes.*;
>
> import static com.knutejohnson.xyzcasinos.translux.Constants.*;
>
> public class SportsWinClient implements Runnable {
>      private final Thread thread;
>
>      private volatile boolean isConnected;
>      private volatile boolean runFlag = true;
>
>      private volatile Socket socket;
>
>      public SportsWinClient() {
>          thread = new Thread(this,"SportsWinClient");
>      }
>
>      public void start() {
>          thread.start();
>      }
>
>      public void run() {
> //        boolean serverFlag = true;
>
>          System.out.println("SportsWinClient: Started");
>          while (runFlag) {
> //            String serverAddress = serverFlag ? SPORTS_WIN_IP_PRIMARY :
> //             SPORTS_WIN_IP_SECONDARY;
>              try {
> //                socket = new Socket(serverAddress,SPORTS_WIN_PORT,
>                  socket = new Socket(SPORTS_WIN_IP,SPORTS_WIN_PORT,
>                   InetAddress.getByName(REMOTE_IP),0);
>                  socket.setKeepAlive(true);
>                  isConnected = true;
>
> ********* I know that the line below is not being executed **********
>
>                  fireConnectionEvent(ConnectionEvent.CONNECTED);
>                  socket.setSoTimeout(3600000);  // one hour timeout
>                  System.out.println("SportsWinClient: Connected");
>                  InputStream is = socket.getInputStream();
>                  InputStreamReader isr = new InputStreamReader(is);
>                  BufferedReader br = new BufferedReader(isr);
>
>                  String str;
>                  while ((str = br.readLine()) != null) {
>                      if (!str.matches("\\d+.*"))  // not a sports record
>                          continue;
>                      SportsBet sb = new SportsBet(str);
>                      SPORTS_BET_MAP.put(sb.betNumber,sb);
>                  }
>
>                  System.out.println("SportsWinClient: End of Stream");
>              } catch (IOException ioe) {
>                  System.out.println("SportsWinClient: " + ioe.toString());
>              } finally {
>                  isConnected = false;
>                  if (socket != null)
>                      try {
>                          socket.close();
>                      } catch (IOException ioe) {
>                          ioe.printStackTrace();
>                      }
>                  fireConnectionEvent(ConnectionEvent.DISCONNECTED);
> //                serverFlag = !serverFlag;
>
> *********** I know that the line below is being executed *************
>
>                  System.out.println("SportsWinClient: Disconnected");
>              }
>              // stop interrupts this thread so this will be bypassed on
> a stop
>              try {
>                  Thread.sleep(10000);
>              } catch (InterruptedException ie) { }
>          }
>          System.out.println("SportsWinClient: Stopping");
>      }
>
>      public void disconnect() {
>          if (isConnected())
>              if (socket != null)
>                  try {
>                      socket.close();
>                  } catch (IOException ioe) {
>                      ioe.printStackTrace();
>                  }
>      }
>
>      public void stop() {
>          runFlag = false;
>          thread.interrupt();
>          if (socket != null)
>              try {
>                  socket.close();
>              } catch (IOException ioe) {
>                  ioe.printStackTrace();
>              }
>      }
>
>      public boolean isConnected() {
>          return isConnected;
>      }
>
>      private final java.util.List<ConnectionListener> connectionListeners =
>       new ArrayList<ConnectionListener>();
>
>      public synchronized void addConnectionListener(ConnectionListener cl) {
>          connectionListeners.add(cl);
>      }
>
>      public synchronized void
> removeConnectionListener(ConnectionListener cl) {
>          connectionListeners.remove(cl);
>      }
>
>      private synchronized void fireConnectionEvent(int id) {
>          ConnectionEvent ce = new ConnectionEvent(this,id);
>
>          for (ConnectionListener listener : connectionListeners)
>              listener.connState(ce);
>      }
>
> }
>
> --
>
> Knute Johnson

Like Steven I would suggest you add Catch Throwable to log the
information about any unexpected happening and make sure that you get
it from the log when it actually occurs. It should give you a clear
indication what to look for to solve the issue.

Kind regards, Paka

[toc] | [prev] | [next] | [standalone]

#12637

On 3/3/2012 3:32 AM, Paka Small wrote:
> On 1 mrt, 20:49, Knute Johnson<nos...@knutejohnson.com>  wrote:
>> I'm having a problem in some production code that I can't figure out.
>> I'll post the complete actual code below.  This code is running in three
>> places and has the same problem in two of them at the same time.  The
>> other I'm not sure, it may be that the personnel operating it are
>> restarting the program and so don't complain.  This piece of code is a
>> simple client that connects via a Socket to a server.  The server
>> supplies some data and the client reads that data and files it away.  It
>> is supposed to restart itself if there is a connection failure or fault
>> for whatever reason.  The problem is that at some random point in time
>> the Socket disconnects, the code logs the disconnect but never restarts.
>>    It does print the "SportsWinClient Disconnected" message but never
>> executes the "fireconnectionEvent()" method after creating a new Socket.
>>    It doesn't print any Exception message.  I'm not sure how it gets out
>> of the try block without printing the "End of Stream" message or an
>> exception message.
>>
>> The crazy part is that all night long when there is no activity from the
>> server it times out and restarts with no problems.
>>
>> I'm hoping that somebody will see a fault in my code that could cause
>> the failure.  It is not a compile problem so I left the formatting as it is.
>>
>> Thanks for looking.
>>
>> package com.knutejohnson.xyzcasinos.translux;
>>
>> import java.io.*;
>> import java.net.*;
>> import java.util.*;
>>
>> import com.knutejohnson.classes.*;
>>
>> import static com.knutejohnson.xyzcasinos.translux.Constants.*;
>>
>> public class SportsWinClient implements Runnable {
>>       private final Thread thread;
>>
>>       private volatile boolean isConnected;
>>       private volatile boolean runFlag = true;
>>
>>       private volatile Socket socket;
>>
>>       public SportsWinClient() {
>>           thread = new Thread(this,"SportsWinClient");
>>       }
>>
>>       public void start() {
>>           thread.start();
>>       }
>>
>>       public void run() {
>> //        boolean serverFlag = true;
>>
>>           System.out.println("SportsWinClient: Started");
>>           while (runFlag) {
>> //            String serverAddress = serverFlag ? SPORTS_WIN_IP_PRIMARY :
>> //             SPORTS_WIN_IP_SECONDARY;
>>               try {
>> //                socket = new Socket(serverAddress,SPORTS_WIN_PORT,
>>                   socket = new Socket(SPORTS_WIN_IP,SPORTS_WIN_PORT,
>>                    InetAddress.getByName(REMOTE_IP),0);
>>                   socket.setKeepAlive(true);
>>                   isConnected = true;
>>
>> ********* I know that the line below is not being executed **********
>>
>>                   fireConnectionEvent(ConnectionEvent.CONNECTED);
>>                   socket.setSoTimeout(3600000);  // one hour timeout
>>                   System.out.println("SportsWinClient: Connected");
>>                   InputStream is = socket.getInputStream();
>>                   InputStreamReader isr = new InputStreamReader(is);
>>                   BufferedReader br = new BufferedReader(isr);
>>
>>                   String str;
>>                   while ((str = br.readLine()) != null) {
>>                       if (!str.matches("\\d+.*"))  // not a sports record
>>                           continue;
>>                       SportsBet sb = new SportsBet(str);
>>                       SPORTS_BET_MAP.put(sb.betNumber,sb);
>>                   }
>>
>>                   System.out.println("SportsWinClient: End of Stream");
>>               } catch (IOException ioe) {
>>                   System.out.println("SportsWinClient: " + ioe.toString());
>>               } finally {
>>                   isConnected = false;
>>                   if (socket != null)
>>                       try {
>>                           socket.close();
>>                       } catch (IOException ioe) {
>>                           ioe.printStackTrace();
>>                       }
>>                   fireConnectionEvent(ConnectionEvent.DISCONNECTED);
>> //                serverFlag = !serverFlag;
>>
>> *********** I know that the line below is being executed *************
>>
>>                   System.out.println("SportsWinClient: Disconnected");
>>               }
>>               // stop interrupts this thread so this will be bypassed on
>> a stop
>>               try {
>>                   Thread.sleep(10000);
>>               } catch (InterruptedException ie) { }
>>           }
>>           System.out.println("SportsWinClient: Stopping");
>>       }
>>
>>       public void disconnect() {
>>           if (isConnected())
>>               if (socket != null)
>>                   try {
>>                       socket.close();
>>                   } catch (IOException ioe) {
>>                       ioe.printStackTrace();
>>                   }
>>       }
>>
>>       public void stop() {
>>           runFlag = false;
>>           thread.interrupt();
>>           if (socket != null)
>>               try {
>>                   socket.close();
>>               } catch (IOException ioe) {
>>                   ioe.printStackTrace();
>>               }
>>       }
>>
>>       public boolean isConnected() {
>>           return isConnected;
>>       }
>>
>>       private final java.util.List<ConnectionListener>  connectionListeners =
>>        new ArrayList<ConnectionListener>();
>>
>>       public synchronized void addConnectionListener(ConnectionListener cl) {
>>           connectionListeners.add(cl);
>>       }
>>
>>       public synchronized void
>> removeConnectionListener(ConnectionListener cl) {
>>           connectionListeners.remove(cl);
>>       }
>>
>>       private synchronized void fireConnectionEvent(int id) {
>>           ConnectionEvent ce = new ConnectionEvent(this,id);
>>
>>           for (ConnectionListener listener : connectionListeners)
>>               listener.connState(ce);
>>       }
>>
>> }
>>
>> --
>>
>> Knute Johnson
>
> Like Steven I would suggest you add Catch Throwable to log the
> information about any unexpected happening and make sure that you get
> it from the log when it actually occurs. It should give you a clear
> indication what to look for to solve the issue.
>
> Kind regards, Paka

Thanks, that's going in tonight if they stop using it early enough.

-- 

Knute Johnson

[toc] | [prev] | [next] | [standalone]

#12627

Knute Johnson pisze:
> I'm having a problem in some production code that I can't figure out. 

Hi Knute,

I was trying to analyze your code, and currently it might be too 
complicated. The class controls thread starting, stopping, network 
operations, parsing the received lines, notyfying other components, and 
so on... Too many concerns.

If I were you, I would start with refactoring it into manageable chunks. 
For example, in the most general terms, your class has to read lines 
from a server and process them somehow. Why don't you write a small 
class, which does EXACTLY that, and push the remaining issues to other 
classes ?

If you do that, you will be able to unit test each of the parts 
separately. Currently it's rather hard...

See this class for example:


public class Worker implements Runnable {

private final String serverAddress;
private volatile boolean running;

//we will get lines from this fellow...
private LineSource lineSource;

//...and pass them to this guy
private final LineProcessor lineProcessor;

public Worker(String serverAddress, LineProcessor lineProcessor) {
	this.running = true;
	this.serverAddress = serverAddress;
	this.lineProcessor = lineProcessor;
}

@Override
public void run() {
	// does not connect to server, just creates the object and saves 	// 
the server address
	lineSource = new LineSource(serverAddress);

	while (running) {
		String line = null;
		try {
			line = lineSource.getLine();
		} catch (Exception e) {
			// TODO LOG IT. MUST... HAVE... LOGS...
		}

		if (line != null) {
			try {
				lineProcessor.processLine(line);
			} catch (Exception e) {
				// TODO LOG IT. MUST... HAVE... LOGS...
			}
		}
	}

	// ok, we're finished, just close the lineSource
	try {
		lineSource.close();
	} catch (Exception e) {
		// TODO LOG IT. MUST... HAVE... LOGS...
	}
}

public void stop() {
	running = false;
}
}

It is just a starting point, still too messy to be used in production. 
And I did't really look into the subject of blocking operations.

I realize that my solution to your problem is an indirect one, but hey, 
it just took five minutes to write this code.

Have a nice day,
Marcin Jaskolski

[toc] | [prev] | [next] | [standalone]

#12638

On 3/3/2012 3:45 AM, x wrote:
> Knute Johnson pisze:
>> I'm having a problem in some production code that I can't figure out.
>
> Hi Knute,
>
> I was trying to analyze your code, and currently it might be too
> complicated. The class controls thread starting, stopping, network
> operations, parsing the received lines, notyfying other components, and
> so on... Too many concerns.
>
> If I were you, I would start with refactoring it into manageable chunks.
> For example, in the most general terms, your class has to read lines
> from a server and process them somehow. Why don't you write a small
> class, which does EXACTLY that, and push the remaining issues to other
> classes ?
>
> If you do that, you will be able to unit test each of the parts
> separately. Currently it's rather hard...
>
> See this class for example:
>
>
> public class Worker implements Runnable {
>
> private final String serverAddress;
> private volatile boolean running;
>
> //we will get lines from this fellow...
> private LineSource lineSource;
>
> //...and pass them to this guy
> private final LineProcessor lineProcessor;
>
> public Worker(String serverAddress, LineProcessor lineProcessor) {
> this.running = true;
> this.serverAddress = serverAddress;
> this.lineProcessor = lineProcessor;
> }
>
> @Override
> public void run() {
> // does not connect to server, just creates the object and saves // the
> server address
> lineSource = new LineSource(serverAddress);
>
> while (running) {
> String line = null;
> try {
> line = lineSource.getLine();
> } catch (Exception e) {
> // TODO LOG IT. MUST... HAVE... LOGS...
> }
>
> if (line != null) {
> try {
> lineProcessor.processLine(line);
> } catch (Exception e) {
> // TODO LOG IT. MUST... HAVE... LOGS...
> }
> }
> }
>
> // ok, we're finished, just close the lineSource
> try {
> lineSource.close();
> } catch (Exception e) {
> // TODO LOG IT. MUST... HAVE... LOGS...
> }
> }
>
> public void stop() {
> running = false;
> }
> }
>
> It is just a starting point, still too messy to be used in production.
> And I did't really look into the subject of blocking operations.
>
> I realize that my solution to your problem is an indirect one, but hey,
> it just took five minutes to write this code.
>
> Have a nice day,
> Marcin Jaskolski

Thanks Marcin.  All of this has already been tested many times as 
individual pieces.  It has been running in this condition for almost a 
year now in three sites.

-- 

Knute Johnson

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | comp.lang.java.programmer

csiph-web