[jira] Created: (LANG-501) Add support for background initialization

Add support for background initialization
-----------------------------------------

                 Key: LANG-501
                 URL: https://issues.apache.org/jira/browse/LANG-501
             Project: Commons Lang
          Issue Type: New Feature
            Reporter: Oliver Heger
            Priority: Minor
             Fix For: 3.0


This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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

     [ https://issues.apache.org/jira/browse/LANG-501?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oliver Heger updated LANG-501:
------------------------------

    Attachment: BackgroundInitializer.patch

An implementation of {{BackgroundInitializer}} with an associated unit test. The test makes use of some mock objects, so I added _EasyMock_ as a dependency with scope _test_.

> Add support for background initialization
> -----------------------------------------
>
>                 Key: LANG-501
>                 URL: https://issues.apache.org/jira/browse/LANG-501
>             Project: Commons Lang
>          Issue Type: New Feature
>            Reporter: Oliver Heger
>            Priority: Minor
>             Fix For: 3.0
>
>         Attachments: BackgroundInitializer.patch
>
>
> This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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

    [ https://issues.apache.org/jira/browse/LANG-501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12705830#action_12705830 ]

Sebb commented on LANG-501:
---------------------------

Not sure why getExternalExecutor() is synchronized whereas setExternalExecutor() is not.
Surely the set method should be synch., especially since the field is documented as @GuardedBy("this") - as is future.

If the externalExecutor could be made final - e.g. by dropping the setExternalExecutor() method - then the synch. would not be needed (assuming ExecutorService is immutable).

The method fetchCurrentExecutor() fetches currentExecutor twice and writes it once, but is not synchronized, and is not thread-safe.

InitializationTask.call() needs to synchronize the code in the finally clause otherwise it may not see the correct value of externalExecutor.

> Add support for background initialization
> -----------------------------------------
>
>                 Key: LANG-501
>                 URL: https://issues.apache.org/jira/browse/LANG-501
>             Project: Commons Lang
>          Issue Type: New Feature
>            Reporter: Oliver Heger
>            Priority: Minor
>             Fix For: 3.0
>
>         Attachments: BackgroundInitializer.patch
>
>
> This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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

    [ https://issues.apache.org/jira/browse/LANG-501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12705994#action_12705994 ]

James Carman commented on LANG-501:
-----------------------------------

I would think it'd be nice to keep the external executor "settable", so that this thing could play nicely in Spring-based environments (you could also use constructor-based injection I guess).  

> Add support for background initialization
> -----------------------------------------
>
>                 Key: LANG-501
>                 URL: https://issues.apache.org/jira/browse/LANG-501
>             Project: Commons Lang
>          Issue Type: New Feature
>            Reporter: Oliver Heger
>            Priority: Minor
>             Fix For: 3.0
>
>         Attachments: BackgroundInitializer.patch
>
>
> This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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

     [ https://issues.apache.org/jira/browse/LANG-501?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oliver Heger updated LANG-501:
------------------------------

    Attachment:     (was: BackgroundInitializer.patch)

> Add support for background initialization
> -----------------------------------------
>
>                 Key: LANG-501
>                 URL: https://issues.apache.org/jira/browse/LANG-501
>             Project: Commons Lang
>          Issue Type: New Feature
>            Reporter: Oliver Heger
>            Priority: Minor
>             Fix For: 3.0
>
>
> This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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

     [ https://issues.apache.org/jira/browse/LANG-501?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oliver Heger updated LANG-501:
------------------------------

    Attachment: BackgroundInitializer.patch

Thanks for the feedback.

Of course, {{setExternalExecutor()}} needs to be synchronized. I simply forgot this. At the beginning I had this field final. But then I had the idea that later a {{MultiBackgroundInitializer}} could be added that manages multiple {{BackgroundInitializer}} objects and shares its {{ExecutorService}} between them. For this use case the {{ExecutorService}} needs to be mutable.

I slightly modified the implementation to address your other points: The package private methods are now only called from synchronized blocks, so there is no need for them being synchronized. {{InitializationTask}} is passed a reference to the {{ExecutorService}} it should destroy, so it need not access a member field of the outer class any more.

> Add support for background initialization
> -----------------------------------------
>
>                 Key: LANG-501
>                 URL: https://issues.apache.org/jira/browse/LANG-501
>             Project: Commons Lang
>          Issue Type: New Feature
>            Reporter: Oliver Heger
>            Priority: Minor
>             Fix For: 3.0
>
>         Attachments: BackgroundInitializer.patch
>
>
> This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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

    [ https://issues.apache.org/jira/browse/LANG-501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12706214#action_12706214 ]

Sebb commented on LANG-501:
---------------------------

That's better, however fetchTemporaryExecutor() is not synchronized.

> Add support for background initialization
> -----------------------------------------
>
>                 Key: LANG-501
>                 URL: https://issues.apache.org/jira/browse/LANG-501
>             Project: Commons Lang
>          Issue Type: New Feature
>            Reporter: Oliver Heger
>            Priority: Minor
>             Fix For: 3.0
>
>         Attachments: BackgroundInitializer.patch
>
>
> This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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

    [ https://issues.apache.org/jira/browse/LANG-501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12706567#action_12706567 ]

Oliver Heger commented on LANG-501:
-----------------------------------

I think that {{fetchTemporaryExecutor()}} is not synchronized is not a big problem as the method is package private (and in {{BackgroundInitializer}} it is only called from synchronized blocks). I assume that we have control over the classes in this package and will detect an incorrect use of this method - maybe a bit naive.

Nevertheless the reason for this method and the {{temporaryExecutor}} field was the thought that another specialized {{BackgroundInitializer}} implementation - namely the {{MultiBackgroundInitializer}} mentioned above - could extend this class and override some of the methods to adapt their behavior. But maybe this is not needed. I hope I can put some code together in a couple of days for an initial implementation. In the ideal case we can get rid of {{fetchTemporaryExecutor()}} and simplify the implementation.

> Add support for background initialization
> -----------------------------------------
>
>                 Key: LANG-501
>                 URL: https://issues.apache.org/jira/browse/LANG-501
>             Project: Commons Lang
>          Issue Type: New Feature
>            Reporter: Oliver Heger
>            Priority: Minor
>             Fix For: 3.0
>
>         Attachments: BackgroundInitializer.patch
>
>
> This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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

    [ https://issues.apache.org/jira/browse/LANG-501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12706702#action_12706702 ]

Sebb commented on LANG-501:
---------------------------

I see.

createTask() also calls fetchTemporaryExecutor() and is not synch (and has a possible data race with getExternalExecutor() - maybe that does not matter), but again is package private. If these methods are to be kept, one solution would be to document the synch. requirements.

But a simpler implementation using final fields as far as possible would be easier to analyse and make thread-safe

> Add support for background initialization
> -----------------------------------------
>
>                 Key: LANG-501
>                 URL: https://issues.apache.org/jira/browse/LANG-501
>             Project: Commons Lang
>          Issue Type: New Feature
>            Reporter: Oliver Heger
>            Priority: Minor
>             Fix For: 3.0
>
>         Attachments: BackgroundInitializer.patch
>
>
> This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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

     [ https://issues.apache.org/jira/browse/LANG-501?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oliver Heger updated LANG-501:
------------------------------

    Attachment:     (was: BackgroundInitializer.patch)

> Add support for background initialization
> -----------------------------------------
>
>                 Key: LANG-501
>                 URL: https://issues.apache.org/jira/browse/LANG-501
>             Project: Commons Lang
>          Issue Type: New Feature
>            Reporter: Oliver Heger
>            Priority: Minor
>             Fix For: 3.0
>
>
> This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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

     [ https://issues.apache.org/jira/browse/LANG-501?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oliver Heger updated LANG-501:
------------------------------

    Attachment: BackgroundInitializer.patch

Slightly modified patch for {{BackgroundInitializer}}. The class was re-worked for being a base class for {{MultiBackgroundInitializer}} (patch to follow). It could be simplified a bit, which also makes reasoning about thread-safety easier.

> Add support for background initialization
> -----------------------------------------
>
>                 Key: LANG-501
>                 URL: https://issues.apache.org/jira/browse/LANG-501
>             Project: Commons Lang
>          Issue Type: New Feature
>            Reporter: Oliver Heger
>            Priority: Minor
>             Fix For: 3.0
>
>         Attachments: BackgroundInitializer.patch
>
>
> This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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

     [ https://issues.apache.org/jira/browse/LANG-501?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oliver Heger updated LANG-501:
------------------------------

    Attachment: MultiBackgroundInitializer.patch

An initial implementation of {{MultiBackgroundInitializer}} plus unit test class.

{{MultiBackgroundInitializer}} can be initialized with an arbitrary number of {{BackgroundInitializer}} objects. It executes these objects in background tasks and waits for their completion.

> Add support for background initialization
> -----------------------------------------
>
>                 Key: LANG-501
>                 URL: https://issues.apache.org/jira/browse/LANG-501
>             Project: Commons Lang
>          Issue Type: New Feature
>            Reporter: Oliver Heger
>            Priority: Minor
>             Fix For: 3.0
>
>         Attachments: BackgroundInitializer.patch, MultiBackgroundInitializer.patch
>
>
> This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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

    [ https://issues.apache.org/jira/browse/LANG-501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12772283#action_12772283 ]

Oliver Heger commented on LANG-501:
-----------------------------------

Patches committed in revisions 831586, 831587.

I am going to leave this ticket open for a while because there is still room for improvements or enhancements. For instance, a specialized background initializer could be added that operates on a {{Callable}} object.

> Add support for background initialization
> -----------------------------------------
>
>                 Key: LANG-501
>                 URL: https://issues.apache.org/jira/browse/LANG-501
>             Project: Commons Lang
>          Issue Type: New Feature
>            Reporter: Oliver Heger
>            Priority: Minor
>             Fix For: 3.0
>
>         Attachments: BackgroundInitializer.patch, MultiBackgroundInitializer.patch
>
>
> This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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

     [ https://issues.apache.org/jira/browse/LANG-501?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oliver Heger updated LANG-501:
------------------------------

    Attachment: CallableBackgroundInitializer.patch

Patch for adding a CallableBackgroundInitializer which executes a Callable object in the background thread.

> Add support for background initialization
> -----------------------------------------
>
>                 Key: LANG-501
>                 URL: https://issues.apache.org/jira/browse/LANG-501
>             Project: Commons Lang
>          Issue Type: New Feature
>            Reporter: Oliver Heger
>            Priority: Minor
>             Fix For: 3.0
>
>         Attachments: BackgroundInitializer.patch, CallableBackgroundInitializer.patch, MultiBackgroundInitializer.patch
>
>
> This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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

     [ https://issues.apache.org/jira/browse/LANG-501?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oliver Heger resolved LANG-501.
-------------------------------

    Resolution: Fixed

The patch for CallableBackgroundInitializer was applied in revision 883101.

> Add support for background initialization
> -----------------------------------------
>
>                 Key: LANG-501
>                 URL: https://issues.apache.org/jira/browse/LANG-501
>             Project: Commons Lang
>          Issue Type: New Feature
>            Reporter: Oliver Heger
>            Priority: Minor
>             Fix For: 3.0
>
>         Attachments: BackgroundInitializer.patch, CallableBackgroundInitializer.patch, MultiBackgroundInitializer.patch
>
>
> This is a suggestion to add a {{BackgroundInitializer}} class that  allows initializing an object in a background task. {{BackgroundInitializer}} is a thin wrapper around a {{java.util.concurrent.Future}} object and uses an {{ExecutorService}} for starting a background task that performs initialization.

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