Just to follow up…

Thanks for all the tips here. This has been quite eye-opening. I’ll never look 
at OSGi in the same way again (in a good way)!

Cheers,
=David




> On Aug 2, 2018, at 19:37, Peter Kriens <[email protected]> wrote:
> 
> Yup, it got a bit windy ;-) I put it on my website as a blog since I've no 
> good other place at the moment.
> 
>       http://aqute.biz/2018/08/02/the-service-window.html 
> <http://aqute.biz/2018/08/02/the-service-window.html>
> 
> Let me know if things are unclear. Kind regards,
> 
>       Peter Kriens
> 
> 
>> On 2 Aug 2018, at 11:58, David Leangen via osgi-dev <[email protected] 
>> <mailto:[email protected]>> wrote:
>> 
>> 
>> Wow! That is a lot to digest.
>> 
>> I’ll need to get back to you in a few days/weeks/months/years. :-D
>> 
>> Thanks so much!!
>> 
>> 
>> Cheers,
>> =David
>> 
>> 
>> 
>> 
>>> On Aug 2, 2018, at 18:38, Peter Kriens <[email protected] 
>>> <mailto:[email protected]>> wrote:
>>> 
>>> 
>>> ## Keep Passing the Open Windows
>>> 
>>> You did read the classic [v2Archive OSGi enRoute App note][5] about this 
>>> topic? It has been archived by the OSGi to [v2Archive OSGi enRoute web 
>>> site][3]. It handles a lot of similar cases. There is an accompanying 
>>> workspace [v2Archive OSGi enRoute osgi.enroute.examples.concurrency 
>>> <https://github.com/osgi/osgi.enroute.examples.concurrency>][7]
>>> 
>>> Anyway, I am not sure if you want to solve this pragmatic or pure?
>>> 
>>> ## Pragmatic 
>>> 
>>> Pragmatic means there is a tiny chance you hit the window where you check 
>>> if the MyService is unregistered and then use it. If you're really unlucky 
>>> you just hit the unregistration after you checked it but before you can use 
>>> it. It works when the unregistration of MyService is rare and the work is 
>>> long. Yes, it can fail but so can anything so you should be prepared for 
>>> it. 
>>> 
>>> Pragmatic works best as follows:
>>> 
>>>    @Component
>>>    public class MyClass extends Thread {   
>>>       @Reference MyService myService;
>>>    
>>>       @Activate void activate()     { start(); }
>>>       @Deactivate void deactivate()         { interrupt(); }
>>>    
>>>       public void run() {
>>>          while (!isInterrupted()) {
>>>             try {
>>>                 MyResult result = doHardWork();
>>>                 if (!isInterrupted())
>>>                     myService.setResult(result);
>>>             } catch (Exception e) { /* TODO */ }
>>>          }
>>>       }
>>>    }
>>> 
>>> Clearly there is a race condition. 
>>> 
>>> 
>>> 
>>> 
>>> ## Pure 
>>> 
>>> I once had a use case where we had whiteboard listeners that received 
>>> events. The frequency and some not so good event listeners that took too 
>>> much time in their callback. This created a quite long window where it 
>>> could fail so it often did. For that use case I created a special highly 
>>> optimized class that could delay the removal of the listener while it was 
>>> being dispatched. To make it have absolutely minimal overhead was tricky, I 
>>> even made an Alloy model of it that found some design errors. Anyway, 
>>> sometimes you have pick one of the bad sides, this was one where delaying 
>>> the deactivate was worth it.
>>> 
>>> So how would you make this 'purer' by delaying the deactivation until you 
>>> stopped using it? Since the service is still supposed to be valid during 
>>> deactivate we could make the setResult() and the deactivate() methods 
>>> exclude each other. That is, we need to make sure that no interrupt can 
>>> happen when we check for the isInterrupted() and call 
>>> myService.setResult(). We could use heavy locks but synchronized works fine 
>>> for me when you realize some of its caveats:
>>> 
>>> * Short blocks
>>> * Ensure you cannot create deadlocks
>>> 
>>> So there must be an explicit contract that the MyService is not going to 
>>> stay away for a long time nor call lots of other unknown code that could 
>>> cause deadlocks. After all, we're blocking the deactivate() method which is 
>>> very bad practice in general. So you will trade off one purity for another.
>>> 
>>>    @Component
>>>    public class MyClass extends Thread {   
>>>       @Reference MyService myService;
>>>    
>>>       @Activate void activate()                     { start(); }
>>>       @Deactivate synchronized void deactivate()    { interrupt(); }
>>>    
>>>       public void run() {
>>>          while (!isInterrupted()) {
>>>             try {
>>>                 MyResult result = doHardWork();
>>>             synchronized(this) {
>>>                     if (!isInterrupted()) {
>>>                         myService.setResult(result);
>>>                 }
>>>             }
>>>             } catch (Exception e) { /* TODO */ }
>>>          }
>>>       }
>>>    }
>>> 
>>> This guarantees what you want … However (you knew this was coming!) there 
>>> is a reason the service gets deactivated. Even though the _service_ is 
>>> still valid at that point, there is a reason the _service object_ indicated 
>>> its unwillingness to play. For example, if MyService was remoted then the 
>>> connection might have been lost. In general, when you call a service you 
>>> should be prepared that it fails. (That is why you should always take 
>>> exceptions into account even if they're not checked.)
>>> 
>>> ## Better API
>>> 
>>> The best solution is usually to turn the problem around. This clearly can 
>>> only happen when you can influence the API so that is often not a choice. 
>>> If you can, you can pass a Promise to the myService and calculate in the 
>>> background. Clearly that means you keep churning doing the hard work. 
>>> Unless the calculation is very expensive and the unregistration happens 
>>> often, doing the calculation unnecessary should normally have no practical 
>>> concerns. If it is, you might want to consider CompletableFuture instead of 
>>> Promise since it has a cancel() method. (We rejected a cancel since it 
>>> makes the Promise mutable, but admittedly it is useful. However, it has the 
>>> same race issues as we discuss here.)
>>> 
>>>    @Component
>>>    public class MyClass {
>>>    
>>>       @Reference MyService myService;
>>>       @Reference PromiseFactory promiseFactory;
>>> 
>>>       @Activate void activate()                     { 
>>>         Promise<MyResult> result = promiseFactory.submit( this::doHardWork 
>>> );
>>>         myService.setResult( result );
>>>       }
>>>    }
>>> 
>>> This is an example where you see a very weird effect that I first noticed 
>>> in the eighties during my first big OO design. At first you think the 
>>> problem is now moved from MyClass to MyService? I think when you try to 
>>> implement this that you find that the problem mostly _disappeared_. During 
>>> one of the first large systems I designed I kept feeling we were kicking 
>>> the hard problems down the road and we still run into a brick wall. 
>>> However, one day we realized we were done. For some reason the hard 
>>> problems were solved in the structure of the application and not in 
>>> specific code. Weird. However, realizing this I often have to cry a bit 
>>> when I realize how some designs are doing the opposite and make simple 
>>> things complex :-(
>>> 
>>> ## Multiple Results
>>> 
>>> If you have multiple results to deliver you might want to take a look at 
>>> the [OSGi PushStream][1]. When I made the initial design for ASyncStreams 
>>> (feels eons ago :-( ) that inspired the OSGi Push Stream specification  
>>> this was one of the use cases I had in mind. The Push Stream are intended 
>>> to handle all the nasty cases and shield you from them. As a bonus, it 
>>> actually works for multiple receivers as well. Push Streams provide a 
>>> simple low cost backlink to handle the case where the MyService gets 
>>> closed. Haven't looked at where Push Stream's ended up but as far as I know 
>>> they should still be useful when your hard work delivers multiple results. 
>>> Ah well, I wanted to take a look at it anyway since it has been released 
>>> now. Let's see how that would look like:
>>> 
>>>     @Component
>>>     public class ProviderImpl extends Thread {
>>>     
>>>             @Reference PushStreamProvider           psp;
>>>             @Reference MyService                    myService;
>>> 
>>>             volatile SimplePushEventSource<MyResult>        dispatcher;
>>>     
>>>             @Activate void activate() throws Exception {
>>>                     dispatcher = 
>>> psp.createSimpleEventSource(MyResult.class);
>>>                     myService.setResult(dispatcher);
>>>                     start();
>>>             }
>>>     
>>>             @Deactivate void deactivate() {
>>>                     interrupt();
>>>             }
>>>     
>>>             @Override
>>>             public void run() {
>>>                     try {
>>>                             MyResult r = doHardWork();
>>>                             while (!isInterrupted()) {
>>>                                     dispatcher.publish(r);
>>>                                     r = doHardWork();
>>>                             }
>>>                     } finally {
>>>                             dispatcher.close();
>>>                     }
>>>             }
>>>     }
>>> 
>>> ## Use of Executors
>>> 
>>> As a side note. I've been in systems where everybody was mucking around 
>>> with ExecutorServices and it became a mess. In [v2Archive OSGi enRoute][3] 
>>> I always provided an [Executor service][4] that is shared and does proper 
>>> cleanup when the service getter goes away. (The [v2Archive OSGi enRoute 
>>> Scheduler][6] was also very nice for this since it provides Cancelable 
>>> Promises.) Executor Services created statically are horror in OSGi since 
>>> they are completely oblivious of the OSGi dynamics. And in your case they 
>>> are totally unnecessary. The only utility they provide to you is that they 
>>> interrupt the threads. This is trivial to do when you create your own 
>>> thread. (And messages about the expensiveness of threads are highly 
>>> exaggerated.) Even if you use an Executor you can pass the thread.
>>> 
>>>     Deferred<Thread> deferred = new Deferred<>();   
>>>     Promise<MyResult> promiseFactory.submit( () -> {
>>>             deferred.resolve( Thread.currentThread() );
>>> 
>>>             while ( result == null && !Thread 
>>> <http://thread.is/>.currentThread().isInterrupted() {
>>>                 … do some hard work
>>>                 }
>>>                 return result;
>>>     });
>>> 
>>>     // deactivate
>>>     deferred.getPromise().getValue().interrupt();
>>> 
>>> In general, if you go this route, suggest you clearly separate the 
>>> strategies from the code. I.e. make a separate class to capture the 
>>> strategy of handling these things. Worst designs are where these are mixed.
>>> 
>>> ## Disclaimer
>>> 
>>> I guess this became a tad long, I guess I will turn it into a blog.
>>>  
>>> Anyway, usually disclaimer: none of the code has been tested so use it at 
>>> your own peril!
>>> 
>>> Good luck, kind regards,
>>> 
>>>     Peter Kriens
>>> 
>>> [1]: https://osgi.org/specification/osgi.cmpn/7.0.0/util.pushstream.html 
>>> <https://osgi.org/specification/osgi.cmpn/7.0.0/util.pushstream.html>
>>> [2]: 
>>> http://www.plantuml.com/plantuml/png/RP2n2i8m48RtF4NST6WVe4Cj24M7Ka71EII71jjKxYwLlhsXMXghO-w3Z-zFGQoGVTk8QZW1zbQ3J79PNcGc4QwM6524LxXLmwvHH07epX6Zr_mcCo1WsKwU9LIQRQyOn7GAplCDGPa0nmoHfgdud69ekhr2y-pm_ezQEZW6HFzWCDlHyRl5ksXDN6LWsPNaiteIhpUBjk_D2EGRZeVD1PayrdMv4WKu4_xv1G00
>>>  
>>> <http://www.plantuml.com/plantuml/png/RP2n2i8m48RtF4NST6WVe4Cj24M7Ka71EII71jjKxYwLlhsXMXghO-w3Z-zFGQoGVTk8QZW1zbQ3J79PNcGc4QwM6524LxXLmwvHH07epX6Zr_mcCo1WsKwU9LIQRQyOn7GAplCDGPa0nmoHfgdud69ekhr2y-pm_ezQEZW6HFzWCDlHyRl5ksXDN6LWsPNaiteIhpUBjk_D2EGRZeVD1PayrdMv4WKu4_xv1G00>
>>> [3]: https://v2archive.enroute.osgi.org/ 
>>> <https://v2archive.enroute.osgi.org/>
>>> [4]: 
>>> https://github.com/osgi/v2archive.osgi.enroute/tree/master/osgi.enroute.executor.simple.provider
>>>  
>>> <https://github.com/osgi/v2archive.osgi.enroute/tree/master/osgi.enroute.executor.simple.provider>
>>> [5]: https://v2archive.enroute.osgi.org/appnotes/concurrency.html 
>>> <https://v2archive.enroute.osgi.org/appnotes/concurrency.html> 
>>> [6]: 
>>> https://github.com/osgi/v2archive.osgi.enroute/tree/master/osgi.enroute.scheduler.simple.provider
>>>  
>>> <https://github.com/osgi/v2archive.osgi.enroute/tree/master/osgi.enroute.scheduler.simple.provider>
>>> [7]: https://github.com/osgi/osgi.enroute.examples.concurrency 
>>> <https://github.com/osgi/osgi.enroute.examples.concurrency>
>>> 
>>>> 
>>>> On 2 Aug 2018, at 02:01, David Leangen via osgi-dev 
>>>> <[email protected] <mailto:[email protected]>> wrote:
>>>> 
>>>> 
>>>> Hi Tim,
>>>> 
>>>> Thanks, and this is good advice. The example you give is when the thread 
>>>> is in the same component that is being deactivated. In this case, as you 
>>>> show, it is quite trivial to track the activation state of the component 
>>>> in order to shut down the thread.
>>>> 
>>>> In my case, the trouble I am having is that the long-running thread is in 
>>>> a component that is different from the component that is getting 
>>>> deactivated. For instance, building on your example:
>>>> 
>>>> @Component
>>>> public class MyClass {
>>>> 
>>>>    // Note that I am using a STATIC service
>>>>    @Reference private MyService myService;
>>>> 
>>>>    private final AtomicBoolean closed = new AtomicBoolean();
>>>> 
>>>>    @Activate
>>>>    void start() {
>>>>        new Thread(this::longStart).run()
>>>>    }
>>>> 
>>>> 
>>>>    @Deactivate
>>>>    void stop() {
>>>>        closed.set(true);
>>>>    }
>>>> 
>>>>    void longStart() {
>>>>        for(int i = 0; i < 1000000; i++) {
>>>> 
>>>>            // This only works if the service object is not stateful, 
>>>> otherwise we need
>>>>            // to do a check and throw away an intermediate invalidated 
>>>> result
>>>> 
>>>>            // Understood, but unfortunately the service object is stateful.
>>>> 
>>>>            // The problem is that the dependency can be deactivated at any 
>>>> time, and this
>>>>            // is happening before “closed" in this component get set to 
>>>> “true". I do not know how
>>>>            // to detect the deactivation of the dependency. I need to 
>>>> determine this pre-emptively,
>>>>            // not after-the-fact. Otherwise the result will be destructive.
>>>> 
>>>>            doSomethingWithMyService(myService);
>>>> 
>>>>            // Ideally I would like to do something like this:
>>>>            if (myServiceIsStillActive())
>>>>                doSomethingWithMyService(myService);
>>>>        }
>>>>    }
>>>> }
>>>> 
>>>> In the second example, there is a dynamic @Reference, so I see the point 
>>>> of using an AtomicReference. However, I am using a static @Reference, so I 
>>>> doubt that just putting in an AtomicReference will change the timing 
>>>> problem.
>>>> 
>>>> Any thoughts?
>>>> 
>>>> 
>>>> 
>>>> By the way, instead of using a “closed” variable, I am doing something 
>>>> like this:
>>>> 
>>>>     @Activate
>>>>     void activate()
>>>>     {
>>>>         executor = Executors.newSingleThreadExecutor();
>>>>     }
>>>> 
>>>>     void deactivate()
>>>>     {
>>>>         executor.shutdownNow();
>>>>     }
>>>> 
>>>> Then I only need to test for Thread.interrupted(). I assume this has the 
>>>> same effect as having the check for “closed".
>>>> 
>>>> Cheers,
>>>> =David
>>>> 
>>>> 
>>>> 
>>>>> On Aug 1, 2018, at 16:59, Tim Ward <[email protected] 
>>>>> <mailto:[email protected]>> wrote:
>>>>> 
>>>>> Hi David,
>>>>> 
>>>>> In addition to interrupting the worker thread (which is a good idea). 
>>>>> There are a couple of useful things that you can do using the support 
>>>>> from java.util.concurrent. For example, setting a closed state:
>>>>> 
>>>>> 
>>>>> @Component
>>>>> public class MyClass {
>>>>> 
>>>>>    private final AtomicBoolean closed = new AtomicBoolean();
>>>>> 
>>>>>    @Activate
>>>>>    void start() {
>>>>>        new Thread(this::longStart).run()
>>>>>    }
>>>>> 
>>>>> 
>>>>>    @Deactivate
>>>>>    void stop() {
>>>>>        closed.set(true);
>>>>>    }
>>>>> 
>>>>>    void longStart() {
>>>>>        for(int i = 0; i < 1000000; i++) {
>>>>>            if(closed.get()) {
>>>>>                break;
>>>>>            }
>>>>>            doSomething();
>>>>>        }
>>>>>    }
>>>>> }
>>>>> 
>>>>> Also if your references are dynamic then you should treat them carefully
>>>>> 
>>>>> @Component
>>>>> public class MyClass implements MySlowService {
>>>>> 
>>>>>    private final AtomicReference<MyService> myRef = new 
>>>>> AtomicReference<>();
>>>>> 
>>>>>    @Reference(policy=DYNAMIC)
>>>>>    void setReference(MyService service) {
>>>>>        myRef.set(service)
>>>>>    }
>>>>> 
>>>>>    void unsetReference(MyService service) {
>>>>>        // Note that it is *not* safe to just do a set null, see 
>>>>> Compendium 112.5.12
>>>>>        myRef.compareAndSet(service, null);
>>>>>    }
>>>>> 
>>>>>    public void longRunningTask() {
>>>>>        for(int i = 0; i < 1000000; i++) {
>>>>>            // This only works if the service object is not stateful, 
>>>>> otherwise we need
>>>>>            // to do a check and throw away an intermediate invalidated 
>>>>> result
>>>>> 
>>>>>            MyService myService = myRef.get();
>>>>>            doSomethingWithMyService(myService);
>>>>>        }
>>>>>    }
>>>>> }
>>>>> 
>>>>> I hope you find these helpful.
>>>>> 
>>>>> Tim
>>>>> 
>>>>>> On 1 Aug 2018, at 05:44, David Leangen via osgi-dev 
>>>>>> <[email protected] <mailto:[email protected]>> wrote:
>>>>>> 
>>>>>> 
>>>>>> Hi!
>>>>>> 
>>>>>> I am running into a situation where, what I think is happening is:
>>>>>> 
>>>>>> Component A gets instantiated
>>>>>> Component B
>>>>>> - references A
>>>>>> - gets satisfied once A is satisfied 
>>>>>> - kicks off a long-running process when one of its methods are called
>>>>>> - the long-running process is run in a different thread, with a Promise
>>>>>> Component A is no longer satisfied
>>>>>> But
>>>>>> - the long-running process is still running
>>>>>> - the long-running process now references an invalid Component A
>>>>>> - the long-running thread fails because of the invalid state of 
>>>>>> Component A
>>>>>> Component B is no longer satisfied
>>>>>> 
>>>>>> 
>>>>>> So, the long-running component messes things up, but its component has 
>>>>>> not yet shut down even though its process is still happily running in 
>>>>>> another thread.
>>>>>> 
>>>>>> I can think of two possible solutions, but not sure which is best and 
>>>>>> not sure how to implement:
>>>>>> 
>>>>>> 1) Figure out a way to share an ExecutorService between “related” 
>>>>>> components so that when one component 
>>>>>>     shuts down it will signal to the other related components that their 
>>>>>> threads are now interrupted
>>>>>> 
>>>>>> 2) In the long-running process, determine if the component that provides 
>>>>>> the required service
>>>>>>      is still active before continuing with the havoc-wreaking process
>>>>>> 
>>>>>> 
>>>>>> Does this sound about right?
>>>>>> 
>>>>>> How would I actually accomplish either of these?
>>>>>> 
>>>>>> 
>>>>>> Thanks!
>>>>>> =David
>>>>>> 
>>>>>> 
>>>>>> _______________________________________________
>>>>>> OSGi Developer Mail List
>>>>>> [email protected] <mailto:[email protected]>
>>>>>> https://mail.osgi.org/mailman/listinfo/osgi-dev 
>>>>>> <https://mail.osgi.org/mailman/listinfo/osgi-dev>
>>>>> 
>>>> 
>>>> _______________________________________________
>>>> OSGi Developer Mail List
>>>> [email protected] <mailto:[email protected]>
>>>> https://mail.osgi.org/mailman/listinfo/osgi-dev 
>>>> <https://mail.osgi.org/mailman/listinfo/osgi-dev>
>> 
>> _______________________________________________
>> OSGi Developer Mail List
>> [email protected] <mailto:[email protected]>
>> https://mail.osgi.org/mailman/listinfo/osgi-dev
> 

_______________________________________________
OSGi Developer Mail List
[email protected]
https://mail.osgi.org/mailman/listinfo/osgi-dev

Reply via email to