Should sequences be scopes #1
Comments
|
Done |
slfritchie
added a commit
that referenced
this issue
Oct 18, 2018
There are a couple places where we can be notified that a TCP socket peer has closed the connection on us: 1. _event_notify(), via an event from the ASIO thread. 2. _pending_reads(), where we try to read from the socket (via the @pony_os_recv()? FFI call which is partial!) and get an error. If we discover the remote peer's close by path #2, then sometime later the ASIO thread may notify us of the close via path #1. Both paths will call `@pony_asio_event_unsubscribe(event)`. There's a sanity check + `pony_assert(0)` inside of the Pony runtime to prevent double calls to `@pony_asio_event_unsubscribe(event)`. However, `pony_assert()` will only act if using a Pony "debug" build of the runtime. So, in non-debug use (i.e., normal day-to-day use), nobody notices the problem: it's only visible when using a debug build (probably because you're debugging some *other* problem). This commit adds a comments + check to avoid double-calls to `@pony_asio_event_unsubscribe(event)`.
slfritchie
added a commit
that referenced
this issue
Oct 19, 2018
There are a couple places where we can be notified that a TCP socket peer has closed the connection on us: 1. _event_notify(), via an event from the ASIO thread. 2. _pending_reads(), where we try to read from the socket (via the @pony_os_recv()? FFI call which is partial!) and get an error. If we discover the remote peer's close by path #2, then sometime later the ASIO thread may notify us of the close via path #1. Both paths will call `@pony_asio_event_unsubscribe(event)`. There's a sanity check + `pony_assert(0)` inside of the Pony runtime to prevent double calls to `@pony_asio_event_unsubscribe(event)`. However, `pony_assert()` will only act if using a Pony "debug" build of the runtime. So, in non-debug use (i.e., normal day-to-day use), nobody notices the problem: it's only visible when using a debug build (probably because you're debugging some *other* problem). This commit adds a comment + check to avoid double-calls to `@pony_asio_event_unsubscribe(event)`. It's done by adding a new Pony API FFI call, `@pony_asio_event_get_disposable`, that peeks into the event's flag status (and *not* the `flags` argument to `_event_notify()`!) to check if it's safe to unsubscribe. Many thanks to @dipinhora for discussing the merits of how the heck to fix this and to @nisanharamati for being curious about, "Why is this thing sometimes crashing, but only when using a debug build of Pony?"
slfritchie
added a commit
that referenced
this issue
Oct 19, 2018
There are a couple places where we can be notified that a TCP socket peer has closed the connection on us: 1. _event_notify(), via an event from the ASIO thread. 2. _pending_reads(), where we try to read from the socket (via the @pony_os_recv()? FFI call which is partial!) and get an error. If we discover the remote peer's close by path #2, then sometime later the ASIO thread may notify us of the close via path #1. Both paths will call `@pony_asio_event_unsubscribe(event)`. There's a sanity check + `pony_assert(0)` inside of the Pony runtime to prevent double calls to `@pony_asio_event_unsubscribe(event)`. However, `pony_assert()` will only act if using a Pony "debug" build of the runtime. So, in non-debug use (i.e., normal day-to-day use), nobody notices the problem: it's only visible when using a debug build (probably because you're debugging some *other* problem). This commit adds a comment + check to avoid double-calls to `@pony_asio_event_unsubscribe(event)`. It's done by adding a new Pony API FFI call, `@pony_asio_event_get_disposable`, that peeks into the event's flag status (and *not* the `flags` argument to `_event_notify()`!) to check if it's safe to unsubscribe. Many thanks to @dipinhora for discussing the merits of how the heck to fix this and to @nisanharamati for being curious about, "Why is this thing sometimes crashing, but only when using a debug build of Pony?"
slfritchie
added a commit
to slfritchie/ponyc
that referenced
this issue
Oct 27, 2018
There are a couple places where we can be notified that a TCP socket peer has closed the connection on us: 1. _event_notify(), via an event from the ASIO thread. 2. _pending_reads(), where we try to read from the socket (via the @pony_os_recv()? FFI call which is partial!) and get an error. If we discover the remote peer's close by path ponylang#2, then sometime later the ASIO thread may notify us of the close via path ponylang#1. Both paths will call `@pony_asio_event_unsubscribe(event)`. There's a sanity check + `pony_assert(0)` inside of the Pony runtime to prevent double calls to `@pony_asio_event_unsubscribe(event)`. However, `pony_assert()` will only act if using a Pony "debug" build of the runtime. So, in non-debug use (i.e., normal day-to-day use), nobody notices the problem: it's only visible when using a debug build (probably because you're debugging some *other* problem). This commit adds a comment + check to avoid double-calls to `@pony_asio_event_unsubscribe(event)`. It's done by adding a new Pony API FFI call, `@pony_asio_event_get_disposable`, that peeks into the event's flag status (and *not* the `flags` argument to `_event_notify()`!) to check if it's safe to unsubscribe. Many thanks to @dipinhora for discussing the merits of how the heck to fix this and to @nisanharamati for being curious about, "Why is this thing sometimes crashing, but only when using a debug build of Pony?"
aturley
pushed a commit
that referenced
this issue
Nov 6, 2018
* Fix race condition with socket close event from peer There are a couple places where we can be notified that a TCP socket peer has closed the connection on us: 1. _event_notify(), via an event from the ASIO thread. 2. _pending_reads(), where we try to read from the socket (via the @pony_os_recv()? FFI call which is partial!) and get an error. If we discover the remote peer's close by path #2, then sometime later the ASIO thread may notify us of the close via path #1. Both paths will call `@pony_asio_event_unsubscribe(event)`. There's a sanity check + `pony_assert(0)` inside of the Pony runtime to prevent double calls to `@pony_asio_event_unsubscribe(event)`. However, `pony_assert()` will only act if using a Pony "debug" build of the runtime. So, in non-debug use (i.e., normal day-to-day use), nobody notices the problem: it's only visible when using a debug build (probably because you're debugging some *other* problem). This commit adds a comment + check to avoid double-calls to `@pony_asio_event_unsubscribe(event)`. It's done by adding a new Pony API FFI call, `@pony_asio_event_get_disposable`, that peeks into the event's flag status (and *not* the `flags` argument to `_event_notify()`!) to check if it's safe to unsubscribe. Many thanks to @dipinhora for discussing the merits of how the heck to fix this and to @nisanharamati for being curious about, "Why is this thing sometimes crashing, but only when using a debug build of Pony?" * Code review fix: bit masking is not appropriate for ev->flags * Add prototype for pony_asio_event_get_disposable in event.h Fix Windows linking problem
KittyMac
added a commit
to KittyMac/ponyc
that referenced
this issue
Mar 31, 2020
1. it makes additions to how TK_AS is handled. If it detects itself used in specific cases in an if, it inserts a match which returns true or false. 2. If ponylang#1 happens, I walk the AST inside of the if body to find references to this variable. I flag these ast nodes with "use this child of the union type for this reference only", and made changes to reach.c and lookup.c to make them smart enough to treat this as a narrowed scope.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Every sequence AST node currently is a scope with a symbol table attached. I seem to remember that we decided this wasn't a good idea and should be changed. Am I misremembering, was it simply never done or did we decide against it?
The text was updated successfully, but these errors were encountered: