You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "Sebb (JIRA)" <ji...@apache.org> on 2008/01/31 22:45:09 UTC

[jira] Created: (HTTPCLIENT-740) AbstractConnPool constructor calls thread.Start()

AbstractConnPool constructor calls thread.Start()
-------------------------------------------------

                 Key: HTTPCLIENT-740
                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-740
             Project: HttpComponents HttpClient
          Issue Type: Bug
          Components: HttpClient
            Reporter: Sebb


AbstractConnPool constructor calls thread.Start()

Findbugs says:

Constructor invokes Thread.start()

The constructor starts a thread. This is likely to be wrong if the class is ever extended/subclassed, since the thread will be started before the subclass constructor is started.

The class is not final (and the constructor is protected) which suggests that the class is intended to be extended...

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Commented: (HTTPCLIENT-740) AbstractConnPool constructor calls thread.Start()

Posted by "Roland Weber (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCLIENT-740?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12565198#action_12565198 ] 

Roland Weber commented on HTTPCLIENT-740:
-----------------------------------------

Hi Sebastian,

I would prefer to continue with TSCCM refactoring and bringing it to a somewhat consistent state before starting with the goldplating. But if you want to get started on this stuff, just go ahead. Oleg should be able to help you with modifying the Maven build, I'll take care of my local Ant build when necessary. I just hope the jar you mentioned is only a build-time dependency.

cheers,
  Roland


> AbstractConnPool constructor calls thread.Start()
> -------------------------------------------------
>
>                 Key: HTTPCLIENT-740
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-740
>             Project: HttpComponents HttpClient
>          Issue Type: Improvement
>          Components: HttpClient
>            Reporter: Sebb
>            Assignee: Roland Weber
>            Priority: Trivial
>             Fix For: 4.0 Alpha 3
>
>
> AbstractConnPool constructor calls thread.Start()
> Findbugs says:
> Constructor invokes Thread.start()
> The constructor starts a thread. This is likely to be wrong if the class is ever extended/subclassed, since the thread will be started before the subclass constructor is started.
> The class is not final (and the constructor is protected) which suggests that the class is intended to be extended...

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Resolved: (HTTPCLIENT-740) AbstractConnPool constructor calls thread.Start()

Posted by "Roland Weber (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HTTPCLIENT-740?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Roland Weber resolved HTTPCLIENT-740.
-------------------------------------

       Resolution: Fixed
    Fix Version/s: 4.0 Alpha 3
         Assignee: Roland Weber

Hi Sebastian,

the started thread implements a callback when connections or the connection manager are garbage collected. There are no connections that can be GCed before the constructor returns, and the connection manager is hard referenced from the call stack. Hence, the callback cannot be triggered unless somebody puts maliciously stupid code into derived class constructors.

The reason why I start the thread immediately after creating the reference queue is to make sure that the reference queue is being handled. If I split this in two methods, somebody will create the reference queue without starting the thread, thereby creating a memory leak.

Anyway, I moved the code for creating both reference queue and callback thread to an extra method that has to be called after creating the connection pool. I hope this addresses your requirements as well as mine. The price to pay is that derived classes can no longer populate the pool in the constructor if they want to support connection GC, since the reference queue must be known when creating a pool entry.

Please review the change (revision 618019).

cheers,
  Roland


> AbstractConnPool constructor calls thread.Start()
> -------------------------------------------------
>
>                 Key: HTTPCLIENT-740
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-740
>             Project: HttpComponents HttpClient
>          Issue Type: Improvement
>          Components: HttpClient
>            Reporter: Sebb
>            Assignee: Roland Weber
>            Priority: Trivial
>             Fix For: 4.0 Alpha 3
>
>
> AbstractConnPool constructor calls thread.Start()
> Findbugs says:
> Constructor invokes Thread.start()
> The constructor starts a thread. This is likely to be wrong if the class is ever extended/subclassed, since the thread will be started before the subclass constructor is started.
> The class is not final (and the constructor is protected) which suggests that the class is intended to be extended...

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Commented: (HTTPCLIENT-740) AbstractConnPool constructor calls thread.Start()

Posted by "Sebb (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCLIENT-740?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12565195#action_12565195 ] 

Sebb commented on HTTPCLIENT-740:
---------------------------------

Seems OK to me as far as multi-threadedness is concerned.

Probably ought to start documenting fields such as numConnections with the @GuardedBy annotation.
This is not a standard annotation, so the build would need to be updated to depend on the jar.



> AbstractConnPool constructor calls thread.Start()
> -------------------------------------------------
>
>                 Key: HTTPCLIENT-740
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-740
>             Project: HttpComponents HttpClient
>          Issue Type: Improvement
>          Components: HttpClient
>            Reporter: Sebb
>            Assignee: Roland Weber
>            Priority: Trivial
>             Fix For: 4.0 Alpha 3
>
>
> AbstractConnPool constructor calls thread.Start()
> Findbugs says:
> Constructor invokes Thread.start()
> The constructor starts a thread. This is likely to be wrong if the class is ever extended/subclassed, since the thread will be started before the subclass constructor is started.
> The class is not final (and the constructor is protected) which suggests that the class is intended to be extended...

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Commented: (HTTPCLIENT-740) AbstractConnPool constructor calls thread.Start()

Posted by "Sebb (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCLIENT-740?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12565060#action_12565060 ] 

Sebb commented on HTTPCLIENT-740:
---------------------------------

The RefQueueWorker constructor is passed "this" as a parameter, and uses that to call handleReference().

Even if the class is not extended, this is a bug, because the constructor has not finished, and therefore the object may not have been completely constructed.

The "this" reference must not be used by another thread until the constructor returns. The started thread may use it before the constructor returns.

[See p41 of Java Concurrency in Practice for more details]

> AbstractConnPool constructor calls thread.Start()
> -------------------------------------------------
>
>                 Key: HTTPCLIENT-740
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-740
>             Project: HttpComponents HttpClient
>          Issue Type: Improvement
>          Components: HttpClient
>            Reporter: Sebb
>            Priority: Trivial
>
> AbstractConnPool constructor calls thread.Start()
> Findbugs says:
> Constructor invokes Thread.start()
> The constructor starts a thread. This is likely to be wrong if the class is ever extended/subclassed, since the thread will be started before the subclass constructor is started.
> The class is not final (and the constructor is protected) which suggests that the class is intended to be extended...

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Updated: (HTTPCLIENT-740) AbstractConnPool constructor calls thread.Start()

Posted by "Roland Weber (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HTTPCLIENT-740?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Roland Weber updated HTTPCLIENT-740:
------------------------------------

      Priority: Trivial  (was: Major)
    Issue Type: Improvement  (was: Bug)

In this case, it is not a bug. The background thread being started does not depend on the completed initialization of the pool instance. That part of the code needs to be changed anyway, so I'll move it to an extra method when the time comes. I hope that will be enough to silence findbugs.

cheers,
  Roland


> AbstractConnPool constructor calls thread.Start()
> -------------------------------------------------
>
>                 Key: HTTPCLIENT-740
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-740
>             Project: HttpComponents HttpClient
>          Issue Type: Improvement
>          Components: HttpClient
>            Reporter: Sebb
>            Priority: Trivial
>
> AbstractConnPool constructor calls thread.Start()
> Findbugs says:
> Constructor invokes Thread.start()
> The constructor starts a thread. This is likely to be wrong if the class is ever extended/subclassed, since the thread will be started before the subclass constructor is started.
> The class is not final (and the constructor is protected) which suggests that the class is intended to be extended...

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org