Sounds good +1

> On Dec 3, 2024, at 7:37 AM, Jean-Baptiste Onofré <j...@nanthrax.net> wrote:
> 
> Hi folks
> 
> As I need more time to cleanup the ssh channel cleanup for KARAF-7174, I
> propose to do a new karat release without it and do another one as soon as
> it’s ready.
> 
> Thoughts ?
> 
> Regards
> JB
> 
> Le jeu. 14 nov. 2024 à 11:16, Romain Manni-Bucau <rmannibu...@gmail.com> a
> écrit :
> 
>> Hi Matt,
>> 
>> Not sure I checked the right pr but it still uses osgi services instead of
>> plain instances?
>> 
>> Was more thinking to session.onClose(()-> ....) in the command.
>> An optional support can be that if the command implements this api (so not
>> closeable which is too generic) it is autoregistered.
>> 
>> Side note: take care the try catch block must be in the loop and not around
>> when called ;)
>> 
>> Le jeu. 14 nov. 2024 à 20:11, Matt Pavlovich <mattr...@gmail.com> a écrit
>> :
>> 
>>> Hi Romain-
>>> 
>>> See the PR link, there is a discussion about just re-using Closeable
>>> interface as a convention and then doing some session id association as
>> you
>>> described vs whiteboard.
>>> 
>>> Thanks JB!
>>> 
>>> Matt
>>> 
>>>> On Nov 12, 2024, at 2:17 PM, Romain Manni-Bucau <rmannibu...@gmail.com
>>> 
>>> wrote:
>>>> 
>>>> Looks good.
>>>> 
>>>> If it helps, i know in similar apps we just register the cleanup hooks
>>> in a
>>>> session attribute (list or map) and when disconnection or tileout
>> occurs
>>> we
>>>> just run them.
>>>> Doesnt change everything but avoids the need of a whiteboard which
>> often
>>>> has lifecycle complexity (a big pitfall there is that if you drop a
>>> bundle
>>>> registering a cleaner what should happen? Cleaner will likely fail
>> until
>>>> you prevent the bundle to be stopped or uninstalled but you need the
>>>> cleaner to not leak. You dont see it with the whiteboard, you do with
>> an
>>>> attribute and it doesnt prevent a generic helper class doing a lazy
>>> lookup
>>>> or even reflection but it runs and fails as expected if something is
>>> wrong).
>>>> 
>>>> Hope it helps
>>>> 
>>>> Le mar. 12 nov. 2024 à 19:26, Francois Papon <
>>> francois.pa...@openobject.fr>
>>>> a écrit :
>>>> 
>>>>> Hi JB,
>>>>> 
>>>>> From my side it looks good, I checked the PR and I think it's a good
>>> idea.
>>>>> 
>>>>> regards,
>>>>> 
>>>>> François
>>>>> fpa...@apache.org
>>>>> francois.pa...@openobject.fr
>>>>> 
>>>>> Le 12/11/2024 à 17:28, Jean-Baptiste Onofré a écrit :
>>>>>> Hi folks,
>>>>>> 
>>>>>> I started to work onhttps://issues.apache.org/jira/browse/KARAF-7174
>>>>>> (finally :)).
>>>>>> 
>>>>>> Basically, the issue here is the following:
>>>>>> - log:tail command registers a custom PaxAppender to display log
>>>>>> events in the terminal (via the printEvent() method)
>>>>>> - we have a pipe thread created by log:tail command
>>>>>> 
>>>>>> On a local terminal, all is OK: when the user stops log:tail (using
>>>>>> CTRL-C), the thread is stopped, all good.
>>>>>> However, when log:tail is executed from a remote terminal (ssh), then
>>>>>> we have a problem when the ssh client timeout for instance: the
>>>>>> log:tail thread is still running on the Karaf side, resulting in an
>>>>>> accumulation of threads.
>>>>>> 
>>>>>> To prevent this, and address similar issues in the future, I started
>>>>>> to prototype a ChannelResourceCleaner whiteboard.
>>>>>> Basically, a shell command (and generally speaking any class) can
>>>>>> implement ChannelResourceCleaner interface and register this kind of
>>>>>> OSGi service.
>>>>>> The ChannelResourceCleaner interface defines the close() method.
>>>>>> In the Karaf ssh server, we add a session listener that reacts when a
>>>>>> disconnect event happens. When a disconnect happens, the Karaf ssh
>>>>>> server is looking for all ChannelResourceCleaner services and call
>>>>>> close() on each of them.
>>>>>> 
>>>>>> I created a draft PR (still WIP) illustrating this mechanism:
>>>>>> https://github.com/apache/karaf/pull/1884
>>>>>> 
>>>>>> What do you think about this approach ?
>>>>>> 
>>>>>> Thanks !
>>>>>> Regards
>>>>>> JB
>>>>> 
>>> 
>>> 
>> 

Reply via email to