Release 2.29.3

Dmitri Tikhonov · Dmitri Tikhonov · commit 99a1ad0f2458 · 2021-03-03T09:41:42.000-05:00
- [BUGFIX] Do not send RESET_STREAM if writing to stream is already
  finished.
- perf_client: wait for all ACKs before exiting.
- Improve how generated RESET_STREAM is logged.
- Fix compilation in different combos of adv_tick/conn_stats flags.
- Move qpack warning disablement into src/liblsquic/CMakeLists.txt.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,3 +1,12 @@
+2021-03-03
+    - 2.29.3
+    - [BUGFIX] Do not send RESET_STREAM if writing to stream is already
+      finished.
+    - perf_client: wait for all ACKs before exiting.
+    - Improve how generated RESET_STREAM is logged.
+    - Fix compilation in different combos of adv_tick/conn_stats flags.
+    - Move qpack warning disablement into src/liblsquic/CMakeLists.txt.
+
 2021-02-23
     - 2.29.2
     - Fix regression in gQUIC server: bug #234.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -45,11 +45,6 @@ IF (NOT MSVC)
 
 SET(MY_CMAKE_FLAGS "${MY_CMAKE_FLAGS} -Wall -Wextra -Wno-unused-parameter")
 SET(MY_CMAKE_FLAGS "${MY_CMAKE_FLAGS} -fno-omit-frame-pointer")
-INCLUDE(CheckCCompilerFlag)
-CHECK_C_COMPILER_FLAG(-Wno-implicit-fallthrough HAS_NO_IMPLICIT_FALLTHROUGH)
-IF (HAS_NO_IMPLICIT_FALLTHROUGH)
-    SET(MY_CMAKE_FLAGS "${MY_CMAKE_FLAGS} -Wno-implicit-fallthrough")
-ENDIF()
 
 IF(CMAKE_COMPILER_IS_GNUCC AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.9.3)
     SET(MY_CMAKE_FLAGS "${MY_CMAKE_FLAGS} -Wno-missing-field-initializers")
diff --git a/bin/http_client.c b/bin/http_client.c
@@ -785,7 +785,7 @@ http_client_on_read (lsquic_stream_t *stream, lsquic_stream_ctx_t *st_h)
             if (client_ctx->hcc_reset_after_nbytes &&
                 s_stat_downloaded_bytes > client_ctx->hcc_reset_after_nbytes)
             {
-                lsquic_stream_reset(stream, 0x1);
+                lsquic_stream_maybe_reset(stream, 0x1, 1);
                 break;
             }
             /* test retire_cid after some number of read bytes */
diff --git a/bin/md5_client.c b/bin/md5_client.c
@@ -273,7 +273,9 @@ client_file_on_write_buf (lsquic_stream_ctx_t *st_h)
     if (g_reset_stream.stream_id == lsquic_stream_id(st_h->stream) &&
         lseek(st_h->file->fd, 0, SEEK_CUR) >= g_reset_stream.offset)
     {
-        lsquic_stream_reset(st_h->stream, 0x01    /* QUIC_INTERNAL_ERROR */);
+        /* Note: this is an internal function */
+        lsquic_stream_maybe_reset(st_h->stream,
+                0x01 /* QUIC_INTERNAL_ERROR */, 1);
         g_reset_stream.stream_id = 0;   /* Reset only once */
     }
 
diff --git a/bin/perf_client.c b/bin/perf_client.c
@@ -335,6 +335,7 @@ main (int argc, char **argv)
     TAILQ_INIT(&sports);
     prog_init(&s_prog, 0, &sports, &perf_stream_if, NULL);
     s_prog.prog_api.ea_alpn = "perf";
+    s_prog.prog_settings.es_delay_onclose = 1;
 
     while (-1 != (opt = getopt(argc, argv, PROG_OPTS "hp:T:")))
     {
diff --git a/docs/conf.py b/docs/conf.py
@@ -26,7 +26,7 @@
 # The short X.Y version
 version = u'2.29'
 # The full version, including alpha/beta/rc tags
-release = u'2.29.2'
+release = u'2.29.3'
 
 
 # -- General configuration ---------------------------------------------------
diff --git a/docs/devel.rst b/docs/devel.rst
@@ -0,0 +1,37 @@
+Developing lsquic
+=================
+
+Generating Tags
+---------------
+
+Over the years, we have developed a wrapper around `Universal Ctags`_
+to generate convenient tags so that, for example, ``ci_packet_in`` will
+be able to take you to any of its implementations such as
+``full_conn_ci_packet_in()``, ``evanescent_conn_ci_packet_in()``, and
+others.
+
+_Exuberant_ Ctags will work, too, but the more recent and maintained fork
+of it, the _Universal_ Ctags, is preferred.  (If you are on Ubuntu, you
+should clone Universal Ctags from GitHub and compiled it yourself.  The
+version that comes in the Ubuntu package -- at the time of this writing
+-- is so slow as to be considered broken).
+
+The wrapper is ``tools/gen-tags.pl``.  Run it in the source directory:
+
+.. highlight:: bash
+
+::
+
+    sh$ cd lsquic
+    sh$ ./tools/gen-tags.pl
+
+Maintaining Documentation
+-------------------------
+
+Documentation -- the ``*.rst`` files under ``docs/`` should be kept up-to-date
+with changes in the API in ``include/lsquic.h``.
+
+For convenience, tags for the documentation files can be generated by passing
+the ``--docs`` argument to ``tools/gen-tags.pl``.
+
+.. _`Universal Ctags`: https://ctags.io/
diff --git a/docs/index.rst b/docs/index.rst
@@ -66,6 +66,7 @@ Contents
    tutorial
    apiref
    internals
+   devel
    faq
 
 Indices and tables
diff --git a/include/lsquic.h b/include/lsquic.h
@@ -25,7 +25,7 @@ extern "C" {
 
 #define LSQUIC_MAJOR_VERSION 2
 #define LSQUIC_MINOR_VERSION 29
-#define LSQUIC_PATCH_VERSION 2
+#define LSQUIC_PATCH_VERSION 3
 
 /**
  * Engine flags:
diff --git a/src/liblsquic/CMakeLists.txt b/src/liblsquic/CMakeLists.txt
@@ -85,7 +85,13 @@ SET(lsquic_STAT_SRCS
 )
 
 IF(NOT MSVC)
-set_source_files_properties(ls-qpack/lsqpack.c PROPERTIES COMPILE_FLAGS -Wno-uninitialized)
+    SET(QPACK_FLAGS "-Wno-uninitialized")
+    INCLUDE(CheckCCompilerFlag)
+    CHECK_C_COMPILER_FLAG(-Wno-implicit-fallthrough HAS_NO_IMPLICIT_FALLTHROUGH)
+    IF (HAS_NO_IMPLICIT_FALLTHROUGH)
+        SET(QPACK_FLAGS "${QPACK_FLAGS} -Wno-implicit-fallthrough")
+    ENDIF()
+set_source_files_properties(ls-qpack/lsqpack.c PROPERTIES COMPILE_FLAGS ${QPACK_FLAGS})
 ENDIF()
 
 include_directories(ls-qpack)
diff --git a/src/liblsquic/lsquic_engine.c b/src/liblsquic/lsquic_engine.c
@@ -96,7 +96,7 @@
 #define LSQUIC_DEBUG_NEXT_ADV_TICK 1
 #endif
 
-#if LSQUIC_DEBUG_NEXT_ADV_TICK
+#if LSQUIC_DEBUG_NEXT_ADV_TICK || LSQUIC_CONN_STATS
 #include "lsquic_alarmset.h"
 #endif
 
@@ -3161,6 +3161,8 @@ lsquic_engine_earliest_adv_tick (lsquic_engine_t *engine, int *diff)
     lsquic_time_t now, next_time;
 #if LSQUIC_DEBUG_NEXT_ADV_TICK || LSQUIC_CONN_STATS
     struct lsquic_conn *conn;
+#endif
+#if LSQUIC_DEBUG_NEXT_ADV_TICK
     const enum lsq_log_level L = LSQ_LOG_DEBUG;  /* Easy toggle */
 #endif
 
diff --git a/src/liblsquic/lsquic_full_conn.c b/src/liblsquic/lsquic_full_conn.c
@@ -3807,7 +3807,7 @@ full_conn_ci_close (struct lsquic_conn *lconn)
         {
             stream = lsquic_hashelem_getdata(el);
             if (!lsquic_stream_is_critical(stream))
-                lsquic_stream_reset(stream, 0);
+                lsquic_stream_maybe_reset(stream, 0, 1);
         }
         conn->fc_flags |= FC_CLOSING;
         if (!(conn->fc_flags & FC_GOAWAY_SENT))
@@ -3933,7 +3933,7 @@ headers_stream_on_stream_error (void *ctx, lsquic_stream_id_t stream_id)
          * errors.  There does not seem to be a good reason to figure out
          * and send more specific error codes.
          */
-        lsquic_stream_reset_ext(stream, 1, 0);
+        lsquic_stream_maybe_reset(stream, 1, 0);
     }
 }
 
diff --git a/src/liblsquic/lsquic_full_conn_ietf.c b/src/liblsquic/lsquic_full_conn_ietf.c
@@ -2581,8 +2581,11 @@ generate_rst_stream_frame (struct ietf_full_conn *conn,
     lsquic_send_ctl_incr_pack_sz(&conn->ifc_send_ctl, packet_out, sz);
     packet_out->po_frame_types |= 1 << QUIC_FRAME_RST_STREAM;
     lsquic_stream_rst_frame_sent(stream);
-    LSQ_DEBUG("wrote RST: stream %"PRIu64"; offset 0x%"PRIX64"; error code "
+    LSQ_DEBUG("wrote RST: stream %"PRIu64"; offset %"PRIu64"; error code "
               "%"PRIu64, stream->id, stream->tosend_off, stream->error_code);
+    EV_LOG_CONN_EVENT(LSQUIC_LOG_CONN_ID, "generated RESET_STREAM: stream "
+        "%"PRIu64"; offset %"PRIu64"; error code %"PRIu64, stream->id,
+        stream->tosend_off, stream->error_code);
 
     return 1;
 }
@@ -2919,7 +2922,7 @@ ietf_full_conn_ci_close (struct lsquic_conn *lconn)
             stream = lsquic_hashelem_getdata(el);
             sd = (stream->id >> SD_SHIFT) & 1;
             if (SD_BIDI == sd)
-                lsquic_stream_reset(stream, 0);
+                lsquic_stream_maybe_reset(stream, 0, 1);
         }
         conn->ifc_flags |= IFC_CLOSING;
         conn->ifc_send_flags |= SF_SEND_CONN_CLOSE;
diff --git a/src/liblsquic/lsquic_stream.c b/src/liblsquic/lsquic_stream.c
@@ -144,6 +144,9 @@ stream_readable_http_ietf (struct lsquic_stream *stream);
 static ssize_t
 stream_write_buf (struct lsquic_stream *stream, const void *buf, size_t sz);
 
+static void
+stream_reset (struct lsquic_stream *, uint64_t error_code, int do_close);
+
 static size_t
 active_hq_frame_sizes (const struct lsquic_stream *);
 
@@ -1284,7 +1287,7 @@ lsquic_stream_rst_in (lsquic_stream_t *stream, uint64_t offset,
                         (STREAM_RST_SENT|STREAM_SS_SENT|STREAM_FIN_SENT))
                             && !(stream->sm_bflags & SMBF_IETF)
                                     && !(stream->sm_qflags & SMQF_SEND_RST))
-        lsquic_stream_reset_ext(stream, 7 /* QUIC_RST_ACKNOWLEDGEMENT */, 0);
+        stream_reset(stream, 7 /* QUIC_RST_ACKNOWLEDGEMENT */, 0);
 
     stream->stream_flags |= STREAM_RST_RECVD;
 
@@ -1324,7 +1327,7 @@ lsquic_stream_stop_sending_in (struct lsquic_stream *stream,
 
     if (!(stream->stream_flags & (STREAM_RST_SENT|STREAM_FIN_SENT))
                                     && !(stream->sm_qflags & SMQF_SEND_RST))
-        lsquic_stream_reset_ext(stream, 0, 0);
+        stream_reset(stream, 0, 0);
 
     maybe_finish_stream(stream);
     maybe_schedule_call_on_close(stream);
@@ -1767,7 +1770,7 @@ handle_early_read_shutdown_gquic (struct lsquic_stream *stream)
 {
     if (!(stream->stream_flags & STREAM_RST_SENT))
     {
-        lsquic_stream_reset_ext(stream, 7 /* QUIC_STREAM_CANCELLED */, 0);
+        stream_reset(stream, 7 /* QUIC_STREAM_CANCELLED */, 0);
         stream->sm_qflags |= SMQF_WAIT_FIN_OFF;
     }
 }
@@ -1848,7 +1851,7 @@ stream_shutdown_write (lsquic_stream_t *stream)
                 && !(stream->stream_flags & STREAM_HEADERS_SENT))
         {
             LSQ_DEBUG("headers not sent, send a reset");
-            lsquic_stream_reset(stream, 0);
+            stream_reset(stream, 0, 1);
         }
         else if (stream->sm_n_buffered == 0)
         {
@@ -4244,15 +4247,22 @@ lsquic_stream_set_max_send_off (lsquic_stream_t *stream, uint64_t offset)
 
 
 void
-lsquic_stream_reset (lsquic_stream_t *stream, uint64_t error_code)
+lsquic_stream_maybe_reset (struct lsquic_stream *stream, uint64_t error_code,
+                           int do_close)
 {
-    lsquic_stream_reset_ext(stream, error_code, 1);
+    if (!((stream->stream_flags
+            & (STREAM_RST_SENT|STREAM_FIN_SENT|STREAM_U_WRITE_DONE))
+        || (stream->sm_qflags & SMQF_SEND_RST)))
+    {
+        stream_reset(stream, error_code, do_close);
+    }
+    else if (do_close)
+        stream_shutdown_read(stream);
 }
 
 
-void
-lsquic_stream_reset_ext (lsquic_stream_t *stream, uint64_t error_code,
-                         int do_close)
+static void
+stream_reset (struct lsquic_stream *stream, uint64_t error_code, int do_close)
 {
     if ((stream->stream_flags & STREAM_RST_SENT)
                                     || (stream->sm_qflags & SMQF_SEND_RST))
@@ -4599,7 +4609,7 @@ lsquic_stream_refuse_push (lsquic_stream_t *stream)
             && !(stream->stream_flags & STREAM_RST_SENT))
     {
         LSQ_DEBUG("refusing pushed stream: send reset");
-        lsquic_stream_reset_ext(stream, 8 /* QUIC_REFUSED_STREAM */, 1);
+        stream_reset(stream, 8 /* QUIC_REFUSED_STREAM */, 1);
         return 0;
     }
     else
diff --git a/src/liblsquic/lsquic_stream.h b/src/liblsquic/lsquic_stream.h
@@ -510,10 +510,7 @@ void
 lsquic_stream_stream_frame_sent (lsquic_stream_t *);
 
 void
-lsquic_stream_reset (lsquic_stream_t *, uint64_t error_code);
-
-void
-lsquic_stream_reset_ext (lsquic_stream_t *, uint64_t error_code, int close);
+lsquic_stream_maybe_reset (struct lsquic_stream *, uint64_t error_code, int);
 
 void
 lsquic_stream_call_on_close (lsquic_stream_t *);
diff --git a/tests/test_stream.c b/tests/test_stream.c
@@ -1242,7 +1242,7 @@ test_loc_RST_rem_FIN (struct test_objs *tobjs)
     sss = lsquic_stream_sending_state(stream);
     assert(SSS_SEND == sss);
 
-    lsquic_stream_reset(stream, 0);
+    lsquic_stream_maybe_reset(stream, 0, 1);
     ++stream->n_unacked;    /* Fake sending of packet with RST_STREAM */
     assert(!TAILQ_EMPTY(&tobjs->conn_pub.sending_streams));
     assert((stream->sm_qflags & SMQF_SENDING_FLAGS) == SMQF_SEND_RST);
@@ -1535,7 +1535,7 @@ test_unlimited_stream_flush_data (struct test_objs *tobjs)
     lsquic_stream_flush(stream);
     assert(0x4000 == lsquic_conn_cap_avail(cap));
 
-    lsquic_stream_reset(stream, 0xF00DF00D);
+    lsquic_stream_maybe_reset(stream, 0xF00DF00D, 1);
     assert(0x4000 == lsquic_conn_cap_avail(cap));   /* Still unchanged */
 
     lsquic_stream_destroy(stream);
diff --git a/tools/bench/lsqb.sh b/tools/bench/lsqb.sh
diff --git a/tools/gen-rst-tags.pl b/tools/gen-rst-tags.pl
diff --git a/tools/gen-tags.pl b/tools/gen-tags.pl
diff --git a/tools/qlog-parser.py b/tools/qlog-parser.py

Original file line number	Diff line number	Diff line change
`@@ -785,7 +785,7 @@ http_client_on_read (lsquic_stream_t stream, lsquic_stream_ctx_t st_h)`
`785`	`785`	`if (client_ctx->hcc_reset_after_nbytes &&`
`786`	`786`	`s_stat_downloaded_bytes > client_ctx->hcc_reset_after_nbytes)`
`787`	`787`	`{`
`788`		`- lsquic_stream_reset(stream, 0x1);`
	`788`	`+ lsquic_stream_maybe_reset(stream, 0x1, 1);`
`789`	`789`	`break;`
`790`	`790`	`}`
`791`	`791`	`/* test retire_cid after some number of read bytes */`
Original file line number	Diff line number	Diff line change
`@@ -273,7 +273,9 @@ client_file_on_write_buf (lsquic_stream_ctx_t *st_h)`
`273`	`273`	`if (g_reset_stream.stream_id == lsquic_stream_id(st_h->stream) &&`
`274`	`274`	`lseek(st_h->file->fd, 0, SEEK_CUR) >= g_reset_stream.offset)`
`275`	`275`	`{`
`276`		`- lsquic_stream_reset(st_h->stream, 0x01 /* QUIC_INTERNAL_ERROR */);`
	`276`	`+ /* Note: this is an internal function */`
	`277`	`+ lsquic_stream_maybe_reset(st_h->stream,`
	`278`	`+ 0x01 /* QUIC_INTERNAL_ERROR */, 1);`
`277`	`279`	`g_reset_stream.stream_id = 0; /* Reset only once */`
`278`	`280`	`}`
`279`	`281`
Original file line number	Diff line number	Diff line change
`@@ -335,6 +335,7 @@ main (int argc, char **argv)`
`335`	`335`	`TAILQ_INIT(&sports);`
`336`	`336`	`prog_init(&s_prog, 0, &sports, &perf_stream_if, NULL);`
`337`	`337`	`s_prog.prog_api.ea_alpn = "perf";`
	`338`	`+ s_prog.prog_settings.es_delay_onclose = 1;`
`338`	`339`
`339`	`340`	`while (-1 != (opt = getopt(argc, argv, PROG_OPTS "hp:T:")))`
`340`	`341`	`{`