Hello everybody, I want to discuss in this thread the issues the local_route created, showing why my concerns about this late commit were expressed.
Although may not be a big patch it is a major change as it introduces architectural changes in regard to route executions and related SIP messages. As you know openser uses a lot of global variables during routes execution (when I say routes I mean all type of routes: route, failure_route, branch_route, ...). All these variables create the context of the SIP message being processed at that time. So far it was only the context of one SIP messages there, the message that was received on the network. Now we get nested contexts and things get really messed up. The local_route can be triggered because: - there is a command via MI interface - there is a non SIP worker process that sends it (timer process: msilo, presence, etc) - a SIP message that was received is processed and functions that send new messages are called (e.g., m_dump() call from msilo) The third case breaks everything. Let me give an example: - REGISTER is received and its context is created - m_dump is called - local_route is executed for each SIP MESSAGE sent by msilo - the context is still the one from REGISTER for every MESSAGE while should be a new one - changes in the REGISTER context are reflected in MESSAGE context and viceversa To have everything working clear, the context of the REGISTER must be parked and restored after local_route execution. That would be very nice as it is a big step towards asynchronous processing. However, here is a list with what I find subject for discussion, to be broken, not properly behavior or misleading (considering the timeline of last Friday evening): - exit or drop have no effect, misleading because of their meaning - setsflag() broken - resetsflag() broken - issflagset() broken - setbflag() broken - resetbflag() broken - isbflagset() broken - initial scripts flags are lost, last local_route script flags are getting into initial route script flags - assignments of following pseudo variables is broken - $bf/$bF - branch flags - $sf/$sF - script flags - $br - adding branch - usage of following pseudo-variables is broken - $Ts - time stamp - $Tf - formatted time - $br - branch - $bR - all branches - $time(...) PV class from cfgutils is broken - avp_pushto("$br", ...) is broken - avp_pushto("$ru", ...) with many avps for the second parameter is broken - setdebug() is broken - append_branch() is broken - pv_printf(br, ...) (avp_printf(br, ...)) is broken - have no time to check all the source code, but other developers can see if they use that -- the sip_msg->id is changed, so if you call a function that checks it to see if some cached data is still valid because it is same message you processed by the call of same function or another one, once in the initial route, then in the nested local_route, when you need it again in the initial route, the cached value is considered invalid although it is the message in current processing The new code for local_route has some behaviors I don't really like, so I want discussions about: - if the buffer with the SIP message required by the local_route to run cannot be created (no memory, etc), the message is sent without running local route - I would expect to be dropped and error returned upstream - if the new SIP message after executing local_route cannot be printed to buffer, the initial message is sent - I would expect to be dropped and error returned upstream - decide behavior for exit/drop I haven't added description about all these broken situations because it will be long description, but I can provide if someone feels I am not right (and yes, I may not be right). It is what I found so far, but fixing it is no longer simple. Let's see how many of them are valid and decide afterwards what to do. Waiting for your comments. Thanks! Cheers, Daniel -- http://www.asipto.com _______________________________________________ Devel mailing list Devel@lists.openser.org http://lists.openser.org/cgi-bin/mailman/listinfo/devel