Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upPotential zalgo in ports #712
Comments
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
process-bot
Sep 15, 2016
Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!
Here is what to expect next, and if anyone wants to comment, keep these things in mind.
process-bot
commented
Sep 15, 2016
|
Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it! Here is what to expect next, and if anyone wants to comment, keep these things in mind. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
lukewestby
Sep 20, 2016
Member
I dug into this a little bit:
This happens because subs are iterated in a for loop when a message comes in:
https://github.com/elm-lang/core/blob/master/src/Native/Platform.js#L462-L465
for (var i = 0; i < subs.length; i++)
{
subs[i](value);
}If there are 3 callbacks in subs, and subs[1] removes itself from the list in place by unsubscribing, then subs[2] becomes subs[1] and the for loop ends because i is now equal to subs.length but than final callback has never been called. To avoid this other systems that employ a list of subscribers will make a copy of the subscribers array with Array.prototype.slice() at opportune moments. The assumption there is that the subscribers array is generally small so that copying is fast, and I think that's a fair assumption here as well.
lukewestby
commented
Sep 20, 2016
•
edited
Edited 1 time
-
lukewestby
edited Sep 20, 2016 (most recent)
edited
-
Sep 20, 2016 (most recent)
|
I dug into this a little bit: This happens because subs are iterated in a for loop when a message comes in: https://github.com/elm-lang/core/blob/master/src/Native/Platform.js#L462-L465 for (var i = 0; i < subs.length; i++)
{
subs[i](value);
}If there are 3 callbacks in |
lukewestby
added
the
bug
label
Sep 20, 2016
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
lukewestby
Sep 20, 2016
Member
I've already started a fix, but in the mean time here's an sscce
Main.elm
port module Main exposing (..)
import Html
import Html.App
import Time exposing (Time)
type alias Model =
()
port test : Time -> Cmd msg
type Msg
= Tick Time
update : Msg -> Model -> ( Model, Cmd Msg )
update (Tick time) _ =
( ()
, test time
)
subscriptions : Model -> Sub Msg
subscriptions _ =
Time.every Time.second Tick
main : Program Never
main =
Html.App.program
{ view = \_ -> Html.text ""
, update = update
, subscriptions = subscriptions
, init = ( (), Cmd.none )
}index.html
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title></title>
</head>
<body>
<script src="out.js"></script>
<script>
var app = Elm.Main.fullscreen();
app.ports.test.subscribe(function unsub(time) {
console.log('first sub got called with', time);
app.ports.test.unsubscribe(unsub);
});
app.ports.test.subscribe(function (time) {
console.log('second sub got called with', time);
});
</script>
</body>
</html>You should see 'first sub got called with <time A>' and not 'second sub got called with <time A>', and then it should continue with 'second sub got called with <time N>'
|
I've already started a fix, but in the mean time here's an sscce Main.elm port module Main exposing (..)
import Html
import Html.App
import Time exposing (Time)
type alias Model =
()
port test : Time -> Cmd msg
type Msg
= Tick Time
update : Msg -> Model -> ( Model, Cmd Msg )
update (Tick time) _ =
( ()
, test time
)
subscriptions : Model -> Sub Msg
subscriptions _ =
Time.every Time.second Tick
main : Program Never
main =
Html.App.program
{ view = \_ -> Html.text ""
, update = update
, subscriptions = subscriptions
, init = ( (), Cmd.none )
}index.html <!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title></title>
</head>
<body>
<script src="out.js"></script>
<script>
var app = Elm.Main.fullscreen();
app.ports.test.subscribe(function unsub(time) {
console.log('first sub got called with', time);
app.ports.test.unsubscribe(unsub);
});
app.ports.test.subscribe(function (time) {
console.log('second sub got called with', time);
});
</script>
</body>
</html>You should see |
wishfoundry commentedSep 15, 2016
if more than 1 subscriber exists on a port, and if that port attempts to unsubscribe while in an event, there is potential to skip the next listener on that port
https://github.com/elm-lang/core/blob/master/src/Native/Platform.js#L486