Skip to content

Conversation

@irfanHaslanded
Copy link
Contributor

unit-tests can fail if run in an env where TZ is already set. This is because utest.h setup calls setenv("TZ"), but does not call tzset() immediately after that.

This causes tzname, timezone, and daylight global variables to by out of sync from the newly set environment value of "TZ".

To fix this, explicitly call tzset().

It's unlikely that a program would call setenv("TZ") during it's lifetime, and this should ideally only happen in a test env.

unit-tests can fail if run in an env where TZ is already set.
This is because utest.h setup calls setenv("TZ"), but does not call
tzset() immediately after that.

This causes tzname, timezone, and daylight global variables to by out of
sync from the newly set environment value of "TZ".

To fix this, explicitly call tzset().

It's unlikely that a program would call setenv("TZ") during it's
lifetime, and this should ideally only happen in a test env.
@michalvasko
Copy link
Member

I think the idea here was that anything that depends on TZ will call tzset() by itself. What exactly does not work this way?

@irfanHaslanded
Copy link
Contributor Author

irfanHaslanded commented Mar 14, 2025

Without this change, the following failure can be seen:

export TZ="NZST-12:00:00NZDT-13:00:00,M10.1.0,M3.3.0"
make -j all >/dev/null 2>&1  && ctest -R 'yang_types$' -V
25: Test timeout computed to be: 10000000
25: [==========] Running 5 test(s).
25: [ RUN      ] test_data_xml
25: [       OK ] test_data_xml
25: [ RUN      ] test_print
25: [       OK ] test_print
25: [ RUN      ] test_duplicate
25: [  ERROR   ] --- "<l1 xmlns="urn:tests:a">2005-05-26T06:45:15.88888+12:00</l1><l2 xmlns="urn:tests:a" xmlns:pref="urn:tests:a">/pref:l2[.='/pref:l2']</l2>" != "<l1 xmlns="urn:tests:a">2005-05-25T16:45:15.88888-02:00</l1><l2 xmlns="urn:tests:a" xmlns:pref="urn:tests:a">/pref:l2[.='/pref:l2']</l2>"
25: [   LINE   ] --- /home/irfan/opensource/libyang/tests/utests/types/yang_types.c:204: error: Failure!
25: [  FAILED  ] test_duplicate

The reason is setenv() only updates the environment, and the API which call tzset() implicitly as needed, don't "monitor" the environment and have no way of knowing that "TZ" is now changed.

as per the tzset_internal() implementation, there is a simple is_initialized flag. it doesn't monitor changes to the getenv("TZ") value.

static void
tzset_internal (int always)
{
  static int is_initialized;
  const char *tz;

  if (is_initialized && !always)
    return;
  is_initialized = 1;
 
...
}

Only calling tzset() explicitly calls tzset_internal(1) with the always flag set. implicit calls are called with tzset_internal(0), and cannot accommodate a change in TZ env variable.

@michalvasko
Copy link
Member

Right, so with my logic the tzset() call should be somewhere in date_and_time.c plugin when it is canonizing the value, which is then printed. It follows the logic of tzset() calls in ly_time_time2str() that you changed before. Or, the expectation is that tzset() is called always after TZ env variable is changed and it should never be needed to be called explicitly.

I would follow the first logic since TZ may not be changed only in code.

@irfanHaslanded
Copy link
Contributor Author

Or, the expectation is that tzset() is called always after TZ env variable is changed and it should never be needed to be called explicitly.

I was going for this. because if the code changes the "TZ" env var, thread safety and races must also be considered. setenv() etc. are not thread safe. So, the thread which updates the env, must also call tzset() itself.

@michalvasko
Copy link
Member

So then you think the call is not needed at all in ly_time_time2str()? What about a user setting TZ and then printing some date-and-time value, not sure whether the new TZ would be used without a tzset() call.

@irfanHaslanded
Copy link
Contributor Author

So then you think the call is not needed at all in ly_time_time2str()?

No, it is still needed to be called at least once, as per our previous discussion in #2363 (comment).

What about a user setting TZ and then printing some date-and-time value, not sure whether the new TZ would be used without a tzset() call.

A user setting TZ before starting the program will use the set TZ.

IMHO, If the user code changes the TZ value during the program execution, then it must call tzset() explicitly after setenv(), for it to take effect.

Although, the previous solution of always calling tzset(), works even with user calling setenv("TZ"), it was prone to race.

@michalvasko
Copy link
Member

No, it is still needed to be called at least once, as per our previous discussion in #2363 (comment).

Okay, but based on what you said, it does not have to be called in ly_time_time2str(). Because either 1) TZ is set in the code, then it should also call tzset() or 2) a user changes TZ before executing a binary when the TZ will be used without tzset().

@michalvasko
Copy link
Member

Going with this solution, why not completely remove ly_tzset_once()?

It does not seem necessary to call `ly_tzset_once()` from
`ly_time_time2str()` or `ly_time_tz_offset_at()`.

If the user changes TZ env variable, they must explicitly call `tzset()`
to update the global variables `tzname`, `timezone` and `daylight`
@michalvasko michalvasko merged commit 64c0b0f into CESNET:devel Mar 17, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants