NASA-AMMOS / AIT-Core

MIT License
45 stars 27 forks source link

Infinite recursion started by Packet.toJSON() #127

Open aywaldron opened 6 years ago

aywaldron commented 6 years ago

https://github.com/NASA-AMMOS/AIT-Core/blob/196f4c4f175b6caf407ea73776ef9b906b9ea591/ait/core/tlm.py#L444

Here is the beginning of the traceback when Packet.toJSON() is called. It continues forever until the limit for recursion is reached and the process exits. toJSON calls getattr, which tries to evaluate defn.when.eval, which in turn calls getattr through looking for defn.globals, and so getattr continues to be called recursively forever.

Traceback (most recent call last):
  File "size_test.py", line 10, in <module>
    print(packet.toJSON())
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 445, in toJSON
    return { name: getattr(self, name) for name in self._defn.fieldmap }
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 445, in <dictcomp>
    return { name: getattr(self, name) for name in self._defn.fieldmap }
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 351, in __getattr__
    return self._getattr(fieldname)
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 415, in _getattr
    if defn.when is None or defn.when.eval(self):
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 704, in eval
    result  = eval(self._code, packet._defn.globals, context)
  File "<string>", line 1, in <module>
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 487, in __getitem__
    result = self._packet._getattr(name)
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 415, in _getattr
    if defn.when is None or defn.when.eval(self):
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 704, in eval
    result  = eval(self._code, packet._defn.globals, context)
  File "<string>", line 1, in <module>
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 487, in __getitem__
    result = self._packet._getattr(name)
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 415, in _getattr
    if defn.when is None or defn.when.eval(self):
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 704, in eval
    result  = eval(self._code, packet._defn.globals, context)
  File "<string>", line 1, in <module>
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 487, in __getitem__
    result = self._packet._getattr(name)
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 415, in _getattr
    if defn.when is None or defn.when.eval(self):
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 704, in eval
    result  = eval(self._code, packet._defn.globals, context)
  File "<string>", line 1, in <module>
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 487, in __getitem__
    result = self._packet._getattr(name)
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 415, in _getattr
    if defn.when is None or defn.when.eval(self):
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 704, in eval
    result  = eval(self._code, packet._defn.globals, context)
  File "<string>", line 1, in <module>
  File "/Users/awaldron/Projects/ait/ait-core/ait/core/tlm.py", line 487, in __getitem__
    result = self._packet._getattr(name)
seanlu99 commented 5 years ago

Hi Anna, I found the bug for this infinite recursion in the tlm.yaml file. The line "when: product_type == 3" in the "product_type" field for the Ethernet_HS_Packet definition causes "product_type" to continuously check itself for the value.

nttoole commented 5 years ago

I believe I was able to trigger similar behavior with a call to pickle.loads(serialized_tlm), which calls tlm packet's __getattr__ ==> _getattr ==> _assertField ==> _hasattr ==> self.defn. .. ==> _getattr ...

Note: In my test case, I removed the 'when' clause from the tlm.yaml.

  tlm = pickle.loads(srl_tlm)
  File "/Users/nttoole/Development/ait/git_repos/ait_080119/venv/lib/python2.7/site-packages/ait/core/tlm.py", line 428, in __getattr__
    return self._getattr(fieldname)
  File "/Users/nttoole/Development/ait/git_repos/ait_080119/venv/lib/python2.7/site-packages/ait/core/tlm.py", line 479, in _getattr
    self._assertField(fieldname)
  File "/Users/nttoole/Development/ait/git_repos/ait_080119/venv/lib/python2.7/site-packages/ait/core/tlm.py", line 468, in _assertField
    if not self._hasattr(fieldname):
  File "/Users/nttoole/Development/ait/git_repos/ait_080119/venv/lib/python2.7/site-packages/ait/core/tlm.py", line 512, in _hasattr
    fieldname in self._defn.fieldmap or
  File "/Users/nttoole/Development/ait/git_repos/ait_080119/venv/lib/python2.7/site-packages/ait/core/tlm.py", line 428, in __getattr__
    return self._getattr(fieldname)
  File "/Users/nttoole/Development/ait/git_repos/ait_080119/venv/lib/python2.7/site-packages/ait/core/tlm.py", line 479, in _getattr
    self._assertField(fieldname)
MJJoyce commented 5 years ago

@nttoole, when you removed the when clauses did you make sure to delete the pkl files as a sanity check to ensure that a stale telemetry dictionary wasn't being used?

nttoole commented 5 years ago

I am using the pickle dump/load to string methods, so I wouldn't expect any files to be in play. Is there some hidden caching these methods use?

MJJoyce commented 5 years ago

So the toolkit will write out *.pkl files for each of the config dictionary files on bootup. If you look in the directory that AIT_CONFIG points to you'll see those there. It uses these to cache the dictionaries so it doesn't need to reload them. As a precaution against accidentally using these cached versions I'd recommend always deleting the *.pkl files after making a change to one of the dictionaries.

nttoole commented 5 years ago

I also renamed the affected tlm.yaml to tlm2.yaml, and created a config2.yaml that includes tlm2.yaml. Finally udpated AIT_CONFIG to pojnt to config2.yaml. But will try deleting all the PKL files and re-run.

nttoole commented 5 years ago

Same result, and can confirm that tlm2.pkl file was created after the run.