sysapps / telephony

API to manage telephony calls
15 stars 12 forks source link

Dynamically changing arrays could lead to race conditions #81

Open marcoscaceres opened 11 years ago

marcoscaceres commented 11 years ago

There are a few attributes that are currently defined as an array, that could change dynamically during an application's lifecycle, namely:

This could lead to unexpected behavior and race conditions. For example, the user pops out a sim card or a call hangs up while a script is in the middle of a for loop.

I recommend that we change the above to methods that return a sequence instead. That will just return an array that is a copy of the current state of the system as some moment in time. That should prevent strange race conditions from happening.

zolkis commented 11 years ago

Changes in the 'calls' and 'serviceIds' arrays are signaled by events. serviceIds could be made a sequence, but I would not make 'calls' a sequence. emergencyNumbers array is updated when a service appears enabled, but indeed there is no event for that other than when the 'serviceadded' signal is fired, emergencyNumbers is regenerated by implementation (in my implementation). I wonder if should this be the pattern, and should we state it in the spec this way, or should we spec a new event for the 'emergencynumberschanged'.

marcoscaceres commented 11 years ago

The problem with leaving things as an array is that developers can change stuff in those arrays, copy them by reference, etc. That seems really messy. Like:

var calls = navigator.telephony.calls; 
calls.push("whatever"); 

If calls is a sequence, then the developer can request the current list of calls and do whatever they want with the array without affecting the actual call attribute's value... like:

var calls = navigator.telephony.calls(); 
while(calls.length){
   calls.pop().hold();
}
marcoscaceres commented 11 years ago

FWIW, I do think we should add emergencynumberschanged event.

zolkis commented 11 years ago

With the current API it is likely it will be never fired, but yes, we can add.

marcoscaceres commented 11 years ago

I had a really good discussion about sequences over on public-script-coord: http://lists.w3.org/Archives/Public/public-script-coord/2013AprJun/0748.html

I think we are going to have to change to using getCalls(), etc.

zolkis commented 11 years ago

The primary usage of getCalls() is to fetch the initial list of call object references when starting up, then monitor the calls solely by events. Of course, apps can invoke getCalls() any number of time, but not much use. Apps must not write the call array. For adding a call, must dial, and for removing a call, must disconnect.

marcoscaceres commented 11 years ago

So, with getCalls(), the calls array becomes an internal thing (which is a good thing). Then the rest is monitored with events.

For the utility, see the second example in my comment above.

zolkis commented 11 years ago

Could we use Promises returning a sequence of objects/id's? Is it canonical to writePromise<sequence<Call>> ?

marcoscaceres commented 11 years ago

No, there is currently no support for that in WebIDL