$ZSEARCH() calls in a loop work correctly in certain edge cases #228
Comments
nars1
added a commit
to nars1/YottaDB
that referenced
this issue
May 2, 2018
…y glob()) errors, move on to next file name Not doing so could cause incorrect errors being issued from $ZSEARCH() if the file system state changed between the first and later $ZSEARCH() calls for a given search context.
nars1
added a commit
that referenced
this issue
May 2, 2018
…)) errors, move on to next file name Not doing so could cause incorrect errors being issued from $ZSEARCH() if the file system state changed between the first and later $ZSEARCH() calls for a given search context.
ksbhaskar
changed the title
ksbhaskar
changed the title
ksbhaskar
changed the title
nars1
added a commit
to nars1/YottaDB
that referenced
this issue
Jul 23, 2018
…05 into YottaDB mainline In general, most of the conflicts were resolved by incorporating the V6.3-005 changes as long as they can co-exist with concurrent YottaDB changes. In case they cannot co-exist, the V6.3-005 changes were discarded. In a lot of cases, the only conflict was because of gtm/GTM name usage vs ydb/YDB name usage. These conflicts were resolved in a straightforward manner. Some of the conflict resolution though was not straightforward and would benefit from some more comments which are described below. * sr_port/gtm_string.h and sr_unix/util_output.c STRNCPY_LIT and STRNCPY_LIT_FULL macros are now removed as they are no longer used. This is because most of those usages were a source of buffer overflow warnings which meant fixing them to use something else. After incorporating V6.3-005 changes, the only remaining usage of STRNCPY_LIT in util_output.c is replaced with a MEMCPY_LIT (which is equivalent) but does not generate any buffer overflow warning like STRNCPY_LIT did. * sr_port/have_crit.h deferred_exit_handling_check() is an inline function introduced in V63005. But since conflicting changes were made to the equivalent DEFERRED_SIGNAL_HANDLING_CHECK macro in YottaDB and it was hard to retrofit, the new inline function was discarded. * sr_port/hd.mpt With V6.3-005 (GTM-5574), most of the numeric conversion functions use a new percent utility M program %CONVBASEUTIL. Since the changes involve enhancements to the range of numbers supported as well as some correctness fixes those were chosen instead of a conflicting YottaDB change (commit id afbb06e) without giving up the YottaDB-specific enhancement that hexadecimal numbers with a 0x or 0X prefix are supported. * sr_port/line.c In my understanding, TREF(compile_time) is guaranteed to be TRUE while we are in line() so it was not clear to me why a check of whether it was 0 was done in V6.3-005 so that conflicting part has been removed and instead an assert has been added to ensure this understanding is correct. Otherwise the V6.3-005 change was picked so we call show_source_line() only if the CQ_WARNINGS flag is ON instead of calling it with a "warn" parameter that is set to TRUE or FALSE depending on this flag in prior YottaDB versions. I don't think it makes a difference to the user either ways but it seems better to go with the V6.3-005 change for show_source_line() invocation. * sr_port/msg.m Every *errors_ctl.c file (e.g. merrors_ctl.c) now has a new section to list all undocumented messages in that file. This is printed as a LITDEF section with array names as gdeerrors_undocarr[], merrors_undocarr[] etc. This is now incorporated into the YottaDB version of msg.m (which generates the files differently than GT.M since r1.20). * sr_port/mstack_size_init.c and sr_port/mstack_size_init.h V6.3-005 adds a new env var gtm_mstack_crit_threshold. This is now folded into a new entry in sr_port/ydb_logicals_tab.h. And corresponding changes made to this file to use the index of the new entry here. Macro names MSTACK_CRIT_MIN_RANGE, MSTACK_CRIT_MAX_RANGE, MSTACK_CRIT_DEF_RANGE have been renamed to use THRESHOLD instead of RANGE to be in sync with the env var name which has "threshold" (not range) in it. * sr_port/objlabel.h V6.3-005 bumps the GT.M object file format. Now that those changes are incorporated into YottaDB, the YottaDB object file format is bumped from 4 to 5. * sr_port/op_zg1.c The change in V6.3-005 to this module seems to be from GTM-8943 to ensure a ZGOTO O inside a call-in returns to the parent C caller. This was already taken care of in YottaDB#32. So the V6.3-005 change is discarded. * sr_port/parm_pool.c and sr_port/push_parm_src.h The V6.3-005 changes included added comments on function parameters of push_parm(). But since this function had previously been moved to sr_port/push_parm_src.h in YottaDB, the new comments have been moved there instead. * sr_port/view_arg_convert.c The V6.3-005 change to this module used the stringpool as a temporary storage before calling format2zwr(). It had the following reasoning. * here & 2 other places use stringpool because we use format2zwr to ensure the message is graphic * & we don't return from the rts_error, so a fixed or malloc'd location seems even less attractive But I do not understand the above reasoning as we currently use a stack variable (which is neither fixed like a global variable, or malloced) and that storage goes away after the rts_error. And since YottaDB had fixed a lot more issues with VIEW and $VIEW (YottaDB#224) in r1.22, those change are considered and the V6.3-005 changes are discarded. * sr_unix/cli_parse.c V6.3-005 changed this file to use strncpy and pass the entire allocated buffer length which meant the buffer will be zero filled after the input string is copied. Since the allocated buffer length is MAX_LINE (greater than 32Kb), and since input command lines are less than a 100 bytes, that is a lot of zero filling done (almost 32Kb of zero filling) every time the strncpy() call. So the V6.3-005 change was discarded. YottaDB had already addressed the gcc 8.1 compiler warning differently (which is probably what prompted the V6.3-005 change) and that did not have this unnecessary overhead so that was kept as is. * sr_unix/gt_timers.c Conflicts were resolved by changing any place where deferred_timers_check_needed was set to FALSE or TRUE in V6.3-005 to instead use CLEAR_DEFERRED_TIMERS_CHECK_NEEDED or SET_DEFERRED_TIMERS_CHECK_NEEDED macros. * sr_unix/gtmci.c, sr_port/tp_restart.c and sr_unix/gtm_filter_command.c Any new usages of "frame_pointer->flags & SFF_CI" in V6.3-005 were replaced with "frame_pointer->type & SFT_CI" since SFF_CI is no longer present in YottaDB (nixed as part of YottaDB#32). Not yet sure if there are any subtle issues with this change. Will investigate as we find any issues while coming up with test cases for GTM-8877 (the ability to invoke an M filter from a C function) which is what triggered the SFF_CI changes in V6.3-005. * sr_unix/gtmci.c V6.3-005 had some changes to CITPNESTED error. But since this error no longer issued in YottaDB (since nested call-ins are allowed in TP due to YottaDB#188), those changes were discarded. * sr_unix/gtmrecv.c and sr_unix/gtmsource.c The V6.3-005 changes (due to GTM-8954) were discarded as similar changes were already made in YottaDB#210. In fact, with the GT.M V63005 changes, the r110/srcsrv_extfilter_sig11 subtest failed because stdout in source server was still pointing to /dev/null instead of source server log file resulting in messages related to the internal filter stopping not showing up in source server log. No such issues show up with YottaDB#210 so that is kept. * sr_unix/gtmsource.h and sr_unix/gtmsource_inline.h The JPL_PHASE2_WRITE_COMPLETE macro is now changed to an inline function jpl_phase2_write_complete() in V6.3-005. That change is incorporated with minor changes in names of variables etc. (gtm to ydb). * sr_unix/gvcst_init_sysops.c and sr_unix/mu_rndwn_file.c V6.3-005 changed this module to ensure we never use gtmsecshr to flush the db file header in case db has the READ_ONLY characteristic turned ON. This is similar to what was already done (amongst many other changes) in YottaDB#150. So the V6.3-005 change was discarded. * sr_unix/jnl_prc_vector.c V6.3-005 had changed SNPRINTF to use macros JPV_LEN_USER and JPV_LEN_PRCNAM. In YottaDB though, the same code was changed to address a gcc 8.1 compiler warning and that used SIZEOF(pv->jpv_user) and SIZEOF(pv->jpv_prcnam). Both usages are equivalent right now but could become different if the macros are changed inadvertently. The SIZEOF usage is safer since it ensures we never go beyond the allocated memory (i.e. no buffer overflows) so the YottaDB change is kept and the V6.3-005 change is discarded. * sr_unix/jobchild_init.c and sr_x86_64/opp_ciret.s V6.3-005 introduced changes to invoke opp_ciret() (a new assembly routine) in case of x86_64 else invoke ci_ret_code_exit(). Not sure why this was necessary. YottaDB#32 removed the need for ci_ret_code_exit() altogether so it is suspected that makes the new opp_ciret() also unnecessary. Therefore the V6.3-005 change is discarded and the new .s file nixed. * sr_unix/mupip_dump_fhead.c The V6.3-005 change was accepted/incorporated with a small name change (GTM_PATH_MAX -> YDB_PATH_MAX). * sr_unix/mupip_set_file.c The V6.3-005 change was due to GTM-8957. This change was already done (and in a more comprehensive fashion) by YottaDB#150. So the V6.3-005 change was discarded. * sr_unix/op_fnzsearch.c The V6.3-005 change was due to GTM-8990. But YottaDB#228 addresses the same issue in a better fashion in my understanding so the V6.3-005 change was discarded. * sr_unix/rel_crit.c and sr_unix/rel_lock.c DEFERRED_EXIT_HANDLING_CHECK macro in GT.M translates to DEFERRED_SIGNAL_HANDLING_CHECK macro in YottaDB. So if the GT.M macro was removed, the corresponding YottaDB macro was removed etc. The GT.M changes were accepted. * sr_unix/relinkctl.c V6.3-005 used OBJ_PLATFORM_LABEL to generate the relinkctl key file name. Since this macro was long obsoleted in YottaDB, that particular line was removed and the rest of the V6.3-005 changes were accepted. * sr_unix/sleep.h The V6.3-005 change was accepted except E_9 usages were changed to the more descriptive NANOSECS_IN_SEC. * sr_unix/suspend.c The V6.3-005 change was discarded as it moved the set of "suspend_status" before the send_msg_csa. But this variable had been nixed in YottaDB#215 and replaced with a "is_suspended" global variable which is never used anywhere but is there only to indicate the process is suspended. Moving this global variable before the send_msg_csa() should not affect anything (i.e. no recursion because this variable is never used anywhere).
nars1
added a commit
that referenced
this issue
Jul 23, 2018
…05 into YottaDB mainline In general, most of the conflicts were resolved by incorporating the V6.3-005 changes as long as they can co-exist with concurrent YottaDB changes. In case they cannot co-exist, the V6.3-005 changes were discarded. In a lot of cases, the only conflict was because of gtm/GTM name usage vs ydb/YDB name usage. These conflicts were resolved in a straightforward manner. Some of the conflict resolution though was not straightforward and would benefit from some more comments which are described below. * sr_port/gtm_string.h and sr_unix/util_output.c STRNCPY_LIT and STRNCPY_LIT_FULL macros are now removed as they are no longer used. This is because most of those usages were a source of buffer overflow warnings which meant fixing them to use something else. After incorporating V6.3-005 changes, the only remaining usage of STRNCPY_LIT in util_output.c is replaced with a MEMCPY_LIT (which is equivalent) but does not generate any buffer overflow warning like STRNCPY_LIT did. * sr_port/have_crit.h deferred_exit_handling_check() is an inline function introduced in V63005. But since conflicting changes were made to the equivalent DEFERRED_SIGNAL_HANDLING_CHECK macro in YottaDB and it was hard to retrofit, the new inline function was discarded. * sr_port/hd.mpt With V6.3-005 (GTM-5574), most of the numeric conversion functions use a new percent utility M program %CONVBASEUTIL. Since the changes involve enhancements to the range of numbers supported as well as some correctness fixes those were chosen instead of a conflicting YottaDB change (commit id afbb06e) without giving up the YottaDB-specific enhancement that hexadecimal numbers with a 0x or 0X prefix are supported. * sr_port/line.c In my understanding, TREF(compile_time) is guaranteed to be TRUE while we are in line() so it was not clear to me why a check of whether it was 0 was done in V6.3-005 so that conflicting part has been removed and instead an assert has been added to ensure this understanding is correct. Otherwise the V6.3-005 change was picked so we call show_source_line() only if the CQ_WARNINGS flag is ON instead of calling it with a "warn" parameter that is set to TRUE or FALSE depending on this flag in prior YottaDB versions. I don't think it makes a difference to the user either ways but it seems better to go with the V6.3-005 change for show_source_line() invocation. * sr_port/msg.m Every *errors_ctl.c file (e.g. merrors_ctl.c) now has a new section to list all undocumented messages in that file. This is printed as a LITDEF section with array names as gdeerrors_undocarr[], merrors_undocarr[] etc. This is now incorporated into the YottaDB version of msg.m (which generates the files differently than GT.M since r1.20). * sr_port/mstack_size_init.c and sr_port/mstack_size_init.h V6.3-005 adds a new env var gtm_mstack_crit_threshold. This is now folded into a new entry in sr_port/ydb_logicals_tab.h. And corresponding changes made to this file to use the index of the new entry here. Macro names MSTACK_CRIT_MIN_RANGE, MSTACK_CRIT_MAX_RANGE, MSTACK_CRIT_DEF_RANGE have been renamed to use THRESHOLD instead of RANGE to be in sync with the env var name which has "threshold" (not range) in it. * sr_port/objlabel.h V6.3-005 bumps the GT.M object file format. Now that those changes are incorporated into YottaDB, the YottaDB object file format is bumped from 4 to 5. * sr_port/op_zg1.c The change in V6.3-005 to this module seems to be from GTM-8943 to ensure a ZGOTO O inside a call-in returns to the parent C caller. This was already taken care of in #32. So the V6.3-005 change is discarded. * sr_port/parm_pool.c and sr_port/push_parm_src.h The V6.3-005 changes included added comments on function parameters of push_parm(). But since this function had previously been moved to sr_port/push_parm_src.h in YottaDB, the new comments have been moved there instead. * sr_port/view_arg_convert.c The V6.3-005 change to this module used the stringpool as a temporary storage before calling format2zwr(). It had the following reasoning. * here & 2 other places use stringpool because we use format2zwr to ensure the message is graphic * & we don't return from the rts_error, so a fixed or malloc'd location seems even less attractive But I do not understand the above reasoning as we currently use a stack variable (which is neither fixed like a global variable, or malloced) and that storage goes away after the rts_error. And since YottaDB had fixed a lot more issues with VIEW and $VIEW (#224) in r1.22, those change are considered and the V6.3-005 changes are discarded. * sr_unix/cli_parse.c V6.3-005 changed this file to use strncpy and pass the entire allocated buffer length which meant the buffer will be zero filled after the input string is copied. Since the allocated buffer length is MAX_LINE (greater than 32Kb), and since input command lines are less than a 100 bytes, that is a lot of zero filling done (almost 32Kb of zero filling) every time the strncpy() call. So the V6.3-005 change was discarded. YottaDB had already addressed the gcc 8.1 compiler warning differently (which is probably what prompted the V6.3-005 change) and that did not have this unnecessary overhead so that was kept as is. * sr_unix/gt_timers.c Conflicts were resolved by changing any place where deferred_timers_check_needed was set to FALSE or TRUE in V6.3-005 to instead use CLEAR_DEFERRED_TIMERS_CHECK_NEEDED or SET_DEFERRED_TIMERS_CHECK_NEEDED macros. * sr_unix/gtmci.c, sr_port/tp_restart.c and sr_unix/gtm_filter_command.c Any new usages of "frame_pointer->flags & SFF_CI" in V6.3-005 were replaced with "frame_pointer->type & SFT_CI" since SFF_CI is no longer present in YottaDB (nixed as part of #32). Not yet sure if there are any subtle issues with this change. Will investigate as we find any issues while coming up with test cases for GTM-8877 (the ability to invoke an M filter from a C function) which is what triggered the SFF_CI changes in V6.3-005. * sr_unix/gtmci.c V6.3-005 had some changes to CITPNESTED error. But since this error no longer issued in YottaDB (since nested call-ins are allowed in TP due to #188), those changes were discarded. * sr_unix/gtmrecv.c and sr_unix/gtmsource.c The V6.3-005 changes (due to GTM-8954) were discarded as similar changes were already made in #210. In fact, with the GT.M V63005 changes, the r110/srcsrv_extfilter_sig11 subtest failed because stdout in source server was still pointing to /dev/null instead of source server log file resulting in messages related to the internal filter stopping not showing up in source server log. No such issues show up with #210 so that is kept. * sr_unix/gtmsource.h and sr_unix/gtmsource_inline.h The JPL_PHASE2_WRITE_COMPLETE macro is now changed to an inline function jpl_phase2_write_complete() in V6.3-005. That change is incorporated with minor changes in names of variables etc. (gtm to ydb). * sr_unix/gvcst_init_sysops.c and sr_unix/mu_rndwn_file.c V6.3-005 changed this module to ensure we never use gtmsecshr to flush the db file header in case db has the READ_ONLY characteristic turned ON. This is similar to what was already done (amongst many other changes) in #150. So the V6.3-005 change was discarded. * sr_unix/jnl_prc_vector.c V6.3-005 had changed SNPRINTF to use macros JPV_LEN_USER and JPV_LEN_PRCNAM. In YottaDB though, the same code was changed to address a gcc 8.1 compiler warning and that used SIZEOF(pv->jpv_user) and SIZEOF(pv->jpv_prcnam). Both usages are equivalent right now but could become different if the macros are changed inadvertently. The SIZEOF usage is safer since it ensures we never go beyond the allocated memory (i.e. no buffer overflows) so the YottaDB change is kept and the V6.3-005 change is discarded. * sr_unix/jobchild_init.c and sr_x86_64/opp_ciret.s V6.3-005 introduced changes to invoke opp_ciret() (a new assembly routine) in case of x86_64 else invoke ci_ret_code_exit(). Not sure why this was necessary. #32 removed the need for ci_ret_code_exit() altogether so it is suspected that makes the new opp_ciret() also unnecessary. Therefore the V6.3-005 change is discarded and the new .s file nixed. * sr_unix/mupip_dump_fhead.c The V6.3-005 change was accepted/incorporated with a small name change (GTM_PATH_MAX -> YDB_PATH_MAX). * sr_unix/mupip_set_file.c The V6.3-005 change was due to GTM-8957. This change was already done (and in a more comprehensive fashion) by #150. So the V6.3-005 change was discarded. * sr_unix/op_fnzsearch.c The V6.3-005 change was due to GTM-8990. But #228 addresses the same issue in a better fashion in my understanding so the V6.3-005 change was discarded. * sr_unix/rel_crit.c and sr_unix/rel_lock.c DEFERRED_EXIT_HANDLING_CHECK macro in GT.M translates to DEFERRED_SIGNAL_HANDLING_CHECK macro in YottaDB. So if the GT.M macro was removed, the corresponding YottaDB macro was removed etc. The GT.M changes were accepted. * sr_unix/relinkctl.c V6.3-005 used OBJ_PLATFORM_LABEL to generate the relinkctl key file name. Since this macro was long obsoleted in YottaDB, that particular line was removed and the rest of the V6.3-005 changes were accepted. * sr_unix/sleep.h The V6.3-005 change was accepted except E_9 usages were changed to the more descriptive NANOSECS_IN_SEC. * sr_unix/suspend.c The V6.3-005 change was discarded as it moved the set of "suspend_status" before the send_msg_csa. But this variable had been nixed in #215 and replaced with a "is_suspended" global variable which is never used anywhere but is there only to indicate the process is suspended. Moving this global variable before the send_msg_csa() should not affect anything (i.e. no recursion because this variable is never used anywhere).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Final Release Note
$ZSEARCH() calls in a loop using the same stream and the same pattern work by ignoring edge cases of changes in the underlying files (such as a permission change of a directory that makes a file inaccessible to the process) and pathological cases such as an infinite symbolic link loop. Previously, ZSEARCH() calls in these edge case could incorrectly issue an error (SYSTEM-E-ENO40, SYSTEM-E-ENO13, etc.), with some difference in behavior between Ubuntu 18.04 LTS and prior releases resulting from a change in the underlying API. (#228)
Description
This is an issue noticed in all versions of YottaDB (and GT.M) after upgrading to Ubuntu 18.04.
Prior to the Ubuntu upgrade, the mumps -run command ran with no output (or error).
Draft Release Note
$ZSEARCH() function works correctly on all patterns with wildcards. In prior versions of YottaDB, on newer versions of linux (e.g. Ubuntu 18.04), $ZSEARCH() could incorrectly issue an error in some cases (e.g. SYSTEM-E-ENO40, SYSTEM-E-ENO13 etc.) when the state of at least one of the files matching the pattern changes before the application goes through all the file names (using a loop of $ZSEARCH calls for the same context).
The text was updated successfully, but these errors were encountered: