On Wed, Jan 31, 2018 at 1:49 AM, Mark Thomas <[email protected]> wrote:

> On 31/01/18 08:15, Bruno P. Kinoshita wrote:
> > Not sure if it was intentional.
> >
> > But here's the reason: https://github.com/apache/commons-pool/commit/
> 4a20cdca923bd342360f821d7020538e985d9ec2#diff-
> 38e254894b87bdf9a1758778c7ffd50fL167
> >
> > Instead of a `new Timer("", /* isDaemon */ true)`, now we have an
> implementation of `ThreadFactory` that when it creates new `Thread`s, it
> doesn't set the `setDaemon(true)`. So it just creates a thread with default
> behaviour of daemon set to false.
> >
> > As the previous behaviour was to have the threads as daemon, and there
> doesn't seem to have any arguments for dropping it, we could raise a new
> issue, with a patch, and ping Mark to see what he thinks?
>
> There is a typo in the commit message. The issue is POOL-315. It looks
> like I had finger trouble that day. I've spotted another typo in the
> changelog which I have now fixed.
>
> I don't recall any discussion of whether or not the threads should be
> daemon threads.
>
> Starting with a clean slate, I'd turn this around. What is the argument
> that the threads should be daemon threads?
>
> If the pool is correctly shutdown it should not matter as the evictor
> thread will be stopped at shutdown.
>

Are we sure that our shutdown logic always cleans up the thread-pool no
matter what?

Gary


>
> The only reason I can see for changing back to using a daemon thread is
> that it used to be a daemon thread and that allowed pools to be used
> without shutting them down cleanly.
>
> Overall, I guess I am neutral on changing it.
>
> Mark
>
>
> >
> > Cheers
> > Bruno
> >
> > (ps: the threads have a typo in their names, but it has been fixed in
> the master branch already)
> >
> >
> > ________________________________
> > From: "Wegrzyn, Jakub" <[email protected]>
> > To: Commons Users List <[email protected]>
> > Sent: Wednesday, 31 January 2018 8:54 PM
> > Subject: RE: [POOL] EvictionTimer daemon thread
> >
> >
> >
> > I couldn’t find it either.
> > Pool-351 was a commit message.
> > https://git-wip-us.apache.org/repos/asf?p=commons-pool.git;a=commit;h=
> 4a20cdca923bd342360f821d7020538e985d9ec2
> >
> > Jakub
> >
> >
> > -----Original Message-----
> > From: Gary Gregory [mailto:[email protected]]
> > Sent: Wednesday, January 31, 2018 8:28 AM
> > To: Commons Users List <[email protected]>
> > Subject: Re: [POOL] EvictionTimer daemon thread
> >
> > I cannot find a POOL-351 issue. Can you double check please?
> >
> > Gary
> >
> > On Jan 31, 2018 00:15, "Wegrzyn, Jakub" <[email protected]> wrote:
> >
> >> Hi,
> >>
> >> We want to upgrade dbcp2 from version 2.1.1 to version 2.2.0. It
> >> requires upgrading commons-pool2 from version 2.4.2 to 2.5.0. However,
> >> during the upgrade we encountered a problem.
> >> It seems that commons-pool2 Evictor thread has been changed from
> >> daemon to non-daemon thread (Issue POOL-351, commit:
> >> 4a20cdca923bd342360f821d702053 8e985d9ec2).
> >>
> >> We cannot find any documentation describing the reason or the change
> >> itself. Can you provide more insight why that was changed and add it
> >> to the changes-report.
> >>
> >> Best regards,
> >> Jakub
> >>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [email protected]
> > For additional commands, e-mail: [email protected]
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
>

Reply via email to