Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Non-unique values in alarm-inventory resource leaf-list #6

Open
peckato1 opened this issue Jan 24, 2024 · 0 comments
Open

Non-unique values in alarm-inventory resource leaf-list #6

peckato1 opened this issue Jan 24, 2024 · 0 comments
Assignees
@peckato1

Comments

@peckato1
Copy link
Contributor

Based on a discussion regarding storing duplicate values in leaf-lists make sure that the following code does not insert multiple values in the alarm-inventory. I am afraid it does.

velia/src/ietf-hardware/sysrepo/Sysrepo.cpp

Line 133 in 26b3821

utils::addResourceToAlarmInventoryEntry(m_session, ALARM_SENSOR_MISSING, std::nullopt, resource);

@peckato1 peckato1 self-assigned this Jan 24, 2024
jktjkt added a commit that referenced this issue May 10, 2024
@jktjkt
health: do not rely on the operational DS being valid
5dfa61e 
I've seen the following crash on the real HW recently:

  #0  0x00000000b6c7979c _ZNK7libyang8DataNode6isTermEv (libyang-cpp.so + 0x1b79c)
  #1  0x00000000b6c797e8 _ZNK7libyang8DataNode6asTermEv (libyang-cpp.so + 0x1b7e8)
  #2  0x00000000004927ec operator()<unsigned int, std::basic_string_view<char>, std::optional<std::basic_string_view<char> >, sysrepo::Event, unsigned int> (veliad-health + 0xc7ec)
  #3  0x00000000b6da378c _ZNKSt8functionIFN7sysrepo9ErrorCodeENS0_7SessionEjSt17basic_string_viewIcSt11char_traitsIcEESt8optionalIS6_ENS0_5EventEjEEclES2_jS6_S8_S9_j (libsysrepo-cpp.so + 0x1378c)
  #4  0x00000000b6d1b5a0 sr_module_change_subscribe_enable (libsysrepo.so.7 + 0x135a0)
  #5  0x00000000b6da451c _ZN7sysrepo12Subscription14onModuleChangeERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESt8functionIFNS_9ErrorCodeENS_7SessionEjSt17basic_string_viewIcS4_ESt8optionalISD_ENS_5EventEjEERKSE_IS6_EjNS_16SubscribeOptionsE (libsysrepo-cpp.so + 0x1451c)
  #6  0x00000000b6d9ad04 _ZN7sysrepo7Session14onModuleChangeERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESt8functionIFNS_9ErrorCodeES0_jSt17basic_string_viewIcS4_ESt8optionalISC_ENS_5EventEjEERKSD_IS6_EjNS_16SubscribeOptionsES9_IFvRSt9exceptionEERKSD_INS_10FDHandlingEE (libsysrepo-cpp.so + 0xad04)
  #7  0x0000000000493990 _ZN5velia6health13AlarmsOutputsC2EN7sysrepo7SessionERKSt6vectorISt8functionIFvNS0_5StateEEESaIS8_EE (veliad-health + 0xd990)
  #8  0x000000000048e214 main (veliad-health + 0x8214)
  #9  0x00000000b683e8a8 n/a (libc.so.6 + 0x228a8)
  ELF object binary architecture: ARM

...which happened while I was playing with some service restarts. Since
I do not have anything more complete, my working theory is that
sysrepo-ietf-alarms was dead/dying at the time this code run, and once
that happened, this callback was invoked with information about a
removal of the alarm-summary data. That sounds like something which can
happen, and once that happens, the original code would segfault because
it dereferenced an empty std::optional.

Fix that by wrapping stuff in an if.

Change-Id: If9365fa47f09f1cc0e6cb37a2b149c7a587010c3
jktjkt added a commit that referenced this issue Sep 9, 2024
@jktjkt
health: do not rely on the operational DS being valid
43edf72 
I've seen the following crash on the real HW recently:

  #0  0x00000000b6c7979c _ZNK7libyang8DataNode6isTermEv (libyang-cpp.so + 0x1b79c)
  #1  0x00000000b6c797e8 _ZNK7libyang8DataNode6asTermEv (libyang-cpp.so + 0x1b7e8)
  #2  0x00000000004927ec operator()<unsigned int, std::basic_string_view<char>, std::optional<std::basic_string_view<char> >, sysrepo::Event, unsigned int> (veliad-health + 0xc7ec)
  #3  0x00000000b6da378c _ZNKSt8functionIFN7sysrepo9ErrorCodeENS0_7SessionEjSt17basic_string_viewIcSt11char_traitsIcEESt8optionalIS6_ENS0_5EventEjEEclES2_jS6_S8_S9_j (libsysrepo-cpp.so + 0x1378c)
  #4  0x00000000b6d1b5a0 sr_module_change_subscribe_enable (libsysrepo.so.7 + 0x135a0)
  #5  0x00000000b6da451c _ZN7sysrepo12Subscription14onModuleChangeERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESt8functionIFNS_9ErrorCodeENS_7SessionEjSt17basic_string_viewIcS4_ESt8optionalISD_ENS_5EventEjEERKSE_IS6_EjNS_16SubscribeOptionsE (libsysrepo-cpp.so + 0x1451c)
  #6  0x00000000b6d9ad04 _ZN7sysrepo7Session14onModuleChangeERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESt8functionIFNS_9ErrorCodeES0_jSt17basic_string_viewIcS4_ESt8optionalISC_ENS_5EventEjEERKSD_IS6_EjNS_16SubscribeOptionsES9_IFvRSt9exceptionEERKSD_INS_10FDHandlingEE (libsysrepo-cpp.so + 0xad04)
  #7  0x0000000000493990 _ZN5velia6health13AlarmsOutputsC2EN7sysrepo7SessionERKSt6vectorISt8functionIFvNS0_5StateEEESaIS8_EE (veliad-health + 0xd990)
  #8  0x000000000048e214 main (veliad-health + 0x8214)
  #9  0x00000000b683e8a8 n/a (libc.so.6 + 0x228a8)
  ELF object binary architecture: ARM

...which happened while I was playing with some service restarts. Since
I do not have anything more complete, my working theory is that
sysrepo-ietf-alarms was dead/dying at the time this code run, and once
that happened, this callback was invoked with information about a
removal of the alarm-summary data. That sounds like something which can
happen, and once that happens, the original code would segfault because
it dereferenced an empty std::optional.

Fix that by wrapping stuff in an if.

Change-Id: If9365fa47f09f1cc0e6cb37a2b149c7a587010c3
jktjkt added a commit that referenced this issue Sep 25, 2024
@jktjkt
Use two-phase init to prevent calling a pure virtual
df6365b 
When the background reader starts "too early", before the derived class'
constructor has finished, it results in calling a pure virtual:

 pure virtual method called
 terminate called without an active exception
 Aborted (core dumped)

 #5  0xb6a5e0c8 in std::terminate() () from /lib/libstdc++.so.6
 #6  0xb6a5efec in __cxa_pure_virtual () from /lib/libstdc++.so.6
 #7  0x0050bf34 in operator() (__closure=0x263ca04) at /home/jkt/work/prog/_build/_br-cfb/build/velia-custom/src/ietf-hardware/FspYh.cpp:97
 #8  std::__invoke_impl<void, velia::ietf_hardware::FspYh::FspYh(const std::filesystem::__cxx11::path&, const std::string&, std::shared_ptr<velia::ietf_hardware::TransientI2C>)::<lambda()> > (__f=...)
     at /home/jkt/work/prog/_build/_br-cfb/per-package/velia/host/opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/13.2.0/bits/invoke.h:61
 #9  std::__invoke<velia::ietf_hardware::FspYh::FspYh(const std::filesystem::__cxx11::path&, const std::string&, std::shared_ptr<velia::ietf_hardware::TransientI2C>)::<lambda()> > (__fn=...)
     at /home/jkt/work/prog/_build/_br-cfb/per-package/velia/host/opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/13.2.0/bits/invoke.h:96
 #10 std::thread::_Invoker<std::tuple<velia::ietf_hardware::FspYh::FspYh(const std::filesystem::__cxx11::path&, const std::string&, std::shared_ptr<velia::ietf_hardware::TransientI2C>)::<lambda()> > >::_M_invoke<0> (this=0x263ca04) at /home/jkt/work/prog/_build/_br-cfb/per-package/velia/host/opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/13.2.0/bits/std_thread.h:292
 #11 std::thread::_Invoker<std::tuple<velia::ietf_hardware::FspYh::FspYh(const std::filesystem::__cxx11::path&, const std::string&, std::shared_ptr<velia::ietf_hardware::TransientI2C>)::<lambda()> > >::operator() (this=0x263ca04) at /home/jkt/work/prog/_build/_br-cfb/per-package/velia/host/opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/13.2.0/bits/std_thread.h:299
 #12 std::thread::_State_impl<std::thread::_Invoker<std::tuple<velia::ietf_hardware::FspYh::FspYh(const std::filesystem::__cxx11::path&, const std::string&, std::shared_ptr<velia::ietf_hardware::TransientI2C>)::<lambda()> > > >::_M_run(void) (this=0x263ca00)
     at /home/jkt/work/prog/_build/_br-cfb/per-package/velia/host/opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/13.2.0/bits/std_thread.h:244
 #13 0xb6a8bdb4 in ?? () from /lib/libstdc++.so.6
 #14 0xb687534c in ?? () from /lib/libc.so.6

Change-Id: Ie73d64e5d6d669db3264cfa7b44467a866a21577
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@peckato1 peckato1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@peckato1