Re: [reSIProcate] DUM: Usage of potentially unsafe handles in async *Command() actions

[Scott Godin <sgodin@xxxxxxxxxxxxxxx>]

Yes - I believe it a good candidate as it is fixing a clear defect.

Thanks Daniel,

Scott

On Fri, Feb 22, 2013 at 4:16 PM, Daniel Pocock <daniel@xxxxxxxxxxxxx> wrote:

Francis, Scott - should this be cherry picked for the 1.8 branch (for
v1.8.7)?

Next week I'll start cherry-picking stuff for the 1.8 branch, getting it
in shape for an eventual 1.8.7 release that will include fixes for
crashes, etc

On 26/01/13 03:20, Francis Joanis wrote:
> Done: revision 9956.
>
> Thanks and happy to help,
> Francis
>
>
> On Fri, Jan 25, 2013 at 5:12 PM, Scott Godin <sgodin@xxxxxxxxxxxxxxx> wrote:
>
>> I took a quick look at the concept of what was being done (didn't check
>> every changed line).  It looks good to me.
>>
>> Thanks for taking this on Francis!
>>
>> Regards,
>> Scott
>>
>>
>> On Fri, Jan 25, 2013 at 4:50 PM, Francis Joanis <francis.joanis@xxxxxxxxx>wrote:
>>
>>> Hi again,
>>>
>>> Could someone please review the attached diff before I commit the
>>> changes? My initial tests seem to show that it should work fine.
>>>
>>> The change would affect the DUM adapter commands in:
>>>
>>> - InviteSession
>>> - ClientPagerMessage
>>> - ServerPagerMessage
>>> - ClientRegistration
>>> - ClientPublication
>>> - ClientSubscription
>>> - ServerInviteSession
>>>
>>> I've also fixed a problem with both acceptNITCommand and rejectNITCommand
>>> which ended up recursively calling themselves.
>>>
>>> Thanks!
>>> Francis
>>>
>>>
>>> On Mon, Jan 21, 2013 at 1:40 PM, Francis Joanis <francis.joanis@xxxxxxxxx
>>>> wrote:
>>>
>>>> Hi,
>>>>
>>>>
>>>> Just a quick note... for whatever reason I was under the impression that
>>>> all the DumCommandAdapter commands were already using handles but instead
>>>> they are using session object references. To do the patch I'll also need to
>>>> convert them all from references to handles - this shouldn't cause any
>>>> problem.
>>>>
>>>> Thanks,
>>>> Francis
>>>>
>>>> On Mon, Jan 21, 2013 at 10:08 AM, Scott Godin <sgodin@xxxxxxxxxxxxxxx>wrote:
>>>>
>>>>> I've noticed the potential for that same issues before.  I think you
>>>>> are on the right track here.  It would be great if you could contribute the
>>>>> changes back.  : )
>>>>>
>>>>> Thanks,
>>>>> Scott
>>>>>
>>>>> On Mon, Jan 21, 2013 at 9:58 AM, Francis Joanis <
>>>>> francis.joanis@xxxxxxxxx> wrote:
>>>>>
>>>>>>
>>>>>> Hi guys,
>>>>>>
>>>>>> One of my colleague encountered a crash within DUM where:
>>>>>>
>>>>>> - InviteSession::acceptCommand() was called to perform an async
>>>>>> accept() on a session
>>>>>> - A BYE came in very shortly after and destroyed the same session
>>>>>> prior to the execution of the DumCommand
>>>>>>
>>>>>> Once the stack got to process the enqueued command it got an exception
>>>>>> because the InviteSessionHandle that was cached in the command object was
>>>>>> now invalid.
>>>>>>
>>>>>> A fix would be to ensure that we call handle.isValid() within each
>>>>>> command's executeCommand() method (like AppDialogSet::endCommand()). If the
>>>>>> handle is invalid, we would then do nothing.
>>>>>>
>>>>>> This is somewhat widespread (not limited to InviteSession) so I am
>>>>>> wondering if I might be missing something obvious. If no one objects I'll
>>>>>> start working on a patch.
>>>>>>
>>>>>> Cheers,
>>>>>> Francis
>>>>>>
>>>>>> _______________________________________________
>>>>>> resiprocate-devel mailing list
>>>>>> resiprocate-devel@xxxxxxxxxxxxxxx
>>>>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
>
>
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel@xxxxxxxxxxxxxxx
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel