Attention is currently required from: daniel, pespin.

View Change

1 comment:

• Patchset:

  • Patch Set #2:

    I submitted the srv_destroy fix from daniel here for review (changed order of calls bit): […]

    Thank you both for the input.

    If the aim is to understand what changed the behavior of a patch that used to produce predictable timestamps and then did not, we don't get an answer by changing said patch. That's why I posted the earlier comment here after looking at what happened in between it working and then suddenly not working anymore (essentially) after rebasing (apart from the additional output causing test failure, only talking about the timestamp behavior).

    I did try destroying the 'server link' and reconnecting in between running the server side test (iirc I even added a sleep for a second to see if the problem was the new connection for the other test need variable amounts of time to reconnect) and the client side test and the problem persisted.
    Or maybe something else in the check was failing and I overlooked that.

    Now (using Daniels branch origin/daniel/stream_tests_lsan), the only thing I need to get predictable output is

    ```
              test_segm_ipa_stream_srv_run(tall_test, host, port, srv);
      
              osmo_stream_srv_link_destroy(srv);
      
              srv = osmo_stream_srv_link_create(tall_test);
              if (!srv) {
                      printf("Unable to create server\n");
                      return EXIT_FAILURE;
              }
      
              test_segm_ipa_stream_cli_run(tall_test, host, port, srv);

    ```

    diff:

    ```
    diff --git a/tests/stream/stream_test.c b/tests/stream/stream_test.c
    index 1cf2687..b51dc0f 100644
    --- a/tests/stream/stream_test.c
    +++ b/tests/stream/stream_test.c
    @@ -879,10 +879,17 @@ int main(void)
            test_recon(tall_test, host, port, 8, srv, false);
     
            test_segm_ipa_stream_srv_run(tall_test, host, port, srv);
    -       test_segm_ipa_stream_cli_run(tall_test, host, port, srv);
     
            osmo_stream_srv_link_destroy(srv);
     
    +       srv = osmo_stream_srv_link_create(tall_test);
    +       if (!srv) {
    +               printf("Unable to create server\n");
    +               return EXIT_FAILURE;
    +       }
    +
    +       test_segm_ipa_stream_cli_run(tall_test, host, port, srv);
    +
            talloc_report_full(tall_test, stderr);
     
            printf("Stream tests completed\n");

    ```

    Daniels branch alone *did not* do it for me, not on a fresh clone of libosmo-netif, either.

    I still find it weird that that either change would suddenly be needed. Shouldn't the two tests still be printing predictable timestamps without recreating the server link in between?
    At least that used to be the case.

    There also wasn't really any memleak *affecting* the test process from my understanding, because even if a destroy was missing in between, it's not like the test was continuously spamming allocs (?)

    Maybe it runs differently for the two of you, because you are running it with the sanitizer enabled?

To view, visit change 34224. To unsubscribe, or for help writing mail filters, visit settings.