]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
mgr/PyModule: clear Python exception when NOTIFY_TYPES is missing 66505/head
authorKefu Chai <k.chai@proxmox.com>
Thu, 4 Dec 2025 08:05:27 +0000 (16:05 +0800)
committerKefu Chai <k.chai@proxmox.com>
Thu, 4 Dec 2025 08:30:54 +0000 (16:30 +0800)
Commit 4589c4d8ac5 ("mgr: do not require NOTIFY_TYPES in python modules")
changed load_notify_types() to treat missing NOTIFY_TYPES as non-fatal,
but failed to clear the Python exception state set by PyObject_GetAttrString().

When PyObject_GetAttrString() fails to find an attribute, it:
1. Returns nullptr
2. Sets AttributeError in the interpreter's exception state

The exception state persists across subsequent Python C API calls until
explicitly cleared. This causes unrelated operations to fail with misleading
error messages.

Bug Manifestation:
------------------
In PyModule::load() (line 292), the call sequence is:

1. load_subclass_of("MgrModule", &pClass) - succeeds
2. load_notify_types() - sets AttributeError but returns 0 (success)
3. load_subclass_of("MgrStandbyModule", &pStandbyClass) - FAILS

The failure occurs because PyImport_ImportModule() at line 658 checks
PyErr_Occurred() before operating. When it detects the pending AttributeError
from step 2, it refuses to import and returns NULL immediately.

This causes load_subclass_of() to report:
```
  1 mgr[py] Loading python module 'balancer'
 -1 mgr[py] Module balancer has missing NOTIFY_TYPES member
 -1 mgr[py] Module not found: 'mgr_module'
 -1 mgr[py] AttributeError: type object 'Module' has no attribute 'NOTIFY_TYPES'
```

This error is misleading - mgr_module exists and is importable, but the
import fails due to the uncleared exception from an unrelated operation.

Without this fix, modules without NOTIFY_TYPES may fail to load their
standby class, or cause subsequent module loads to fail entirely with
confusing error messages pointing to the wrong module.

Fix:
----
This patch follows Python's EAFP (Easier to Ask for Forgiveness than
Permission) philosophy by attempting the attribute access and handling the
expected AttributeError, rather than checking preconditions with
PyObject_HasAttrString() (LBYL approach). The EAFP approach:

- Is more pythonic and idiomatic
- Requires only one API call instead of two (more efficient)
- Properly distinguishes expected errors (AttributeError) from unexpected
  errors (e.g., interpreter shutdown, memory errors)
- Handles unexpected errors appropriately by reporting and returning -EINVAL

Implementation:
- Use PyErr_ExceptionMatches() to check for expected AttributeError
- Clear expected exception with PyErr_Clear()
- Report unexpected exceptions via handle_pyerror()

Refs: 4589c4d8ac5 ("mgr: do not require NOTIFY_TYPES in python modules")
Refs: https://tracker.ceph.com/issues/55835
Fixes: https://tracker.ceph.com/issues/70789
Signed-off-by: Kefu Chai <k.chai@proxmox.com>
src/mgr/PyModule.cc

index 89d00d8d00c244ad44fcc9f7f6faa8e39b3063eb..27c8c572df3d62a012794f2f1b1ddd68eec97b65 100644 (file)
@@ -451,8 +451,16 @@ int PyModule::load_notify_types()
 {
   PyObject *ls = PyObject_GetAttrString(pClass, "NOTIFY_TYPES");
   if (ls == nullptr) {
-    dout(10) << "Module " << get_name() << " has no NOTIFY_TYPES member" << dendl;
-    return 0;
+    // NOTIFY_TYPES is optional - clear expected AttributeError
+    if (PyErr_ExceptionMatches(PyExc_AttributeError)) {
+      PyErr_Clear();
+      dout(10) << "Module " << get_name() << " has no NOTIFY_TYPES member" << dendl;
+      return 0;
+    }
+    // Unexpected error - report it
+    derr << "Error getting NOTIFY_TYPES from " << get_name() << ": "
+         << handle_pyerror(true, module_name, "load_notify_types") << dendl;
+    return -EINVAL;
   }
   if (!PyObject_TypeCheck(ls, &PyList_Type)) {
     // Relatively easy mistake for human to make, e.g. defining COMMANDS