Fix broken receive optimization

When a ref is created before performing a receive that will only
receive message containing that ref, there is a compiler optimization
to avoid scanning messages that can't possible contain the newly
created ref.

Magnus Lång pointed out that the implementation of the optimization
is flawed. Exceptions or recursive calls could cause the receive
operation to scan the receive queue from a position beyond the expected
message (that is, the message containing the ref would never be
matched out). See the receive_opt_exception/1 and receive_opt_recursion/1
test cases in receive_SUITE.

It turns out that we can simplify the implementation of the
optimization while fixing the bug (suggested by Magnus Lång). We
actually don't need the c_p->msg.mark field. It is enough to have
c_p->msg.saved_pos; if it is non-zero, it is a valid position in the
message qeueue.  All we need to do is to ensure that we clear
c_p->msg.saved_pos when a receive is exited normally or abnormally.

We can clear c_p->msg.saved_pos in JOIN_MESSAGE(), since it is called
both when leaving a receive because a message matched and because there
was a timeout and the 'after' clause was executed. In addition, we
need to clear c_p->msg.saved_pos when an exception is caught.

https://bugs.erlang.org/browse/ERL-511

1 files changed, 10 insertions, 6 deletions

diff --git a/erts/emulator/beam/erl_message.h b/erts/emulator/beam/erl_message.h
index 9c8cf84e43..a14f4f51d8 100644
--- a/erts/emulator/beam/erl_message.h
+++ b/erts/emulator/beam/erl_message.h
@@ -167,10 +167,9 @@ typedef struct {
     Sint len;            /* queue length */
 
     /*
-     * The following two fields are used by the recv_mark/1 and
+     * The following field is used by the recv_mark/1 and
      * recv_set/1 instructions.
      */
-    BeamInstr* mark;		/* address to rec_loop/2 instruction */
     ErtsMessage** saved_last;	/* saved last pointer */
 } ErlMessageQueue;
 
@@ -236,12 +235,17 @@ typedef struct erl_trace_message_queue__ {
      (p)->msg.len--; \
      if (__mp == NULL) \
          (p)->msg.last = (p)->msg.save; \
-     (p)->msg.mark = 0; \
 } while(0)
 
-/* Reset message save point (after receive match) */
-#define JOIN_MESSAGE(p) \
-     (p)->msg.save = &(p)->msg.first
+/*
+ * Reset message save point (after receive match).
+ * Also invalidate the saved position since it may no
+ * longer be safe to use.
+ */
+#define JOIN_MESSAGE(p) do {                    \
+    (p)->msg.save = &(p)->msg.first;            \
+    (p)->msg.saved_last = 0;                    \
+} while(0)
 
 /* Save current message */
 #define SAVE_MESSAGE(p) \