pyepics icon indicating copy to clipboard operation
pyepics copied to clipboard

MIssing wrapper to ca_add_exception_event

Open NickeZ opened this issue 7 years ago • 13 comments

Hey,

I tried to use replace_printf_handler1 to replace the CA printf handler. The callback is defined as ( const char* ) -> void whereas it should be ( const char *, va_list ) -> int see 2. So what the callback actually gets is the printf-format string, all the values are lost.

According to my brief research python cannot implement va_list types. I guess a small wrapper has to be implemented that calls vsprintf() and then calls the python callback with the formatted string.

What do you think?

Thanks

NickeZ avatar Sep 26 '18 16:09 NickeZ

@NickeZ Hm, I thought that did "just work", but apparently not -- I see that now too. I imagine a va_list might be mapped as a ctypes.c_void_p, but I'm not sure what you would have to do to then populate that. If you have something that does work, or you think might eventually work,I think it would be great!

newville avatar Sep 26 '18 17:09 newville

I don't know much about python-c-bindings unfortunately. It seems like CFUNCTYPE cannot desribe va_list. So I guess a "generic" callback has to be implemented in C that exposes a callback interface that actually can be registered by python functions.

NickeZ avatar Sep 26 '18 19:09 NickeZ

Sorry for the not-so-beatiful code... but something iike below works. I don't know if it is a valid solution for pyepics to depend on an dll though..

libmy_lib.so:

#include <stdarg.h>
#include <stdlib.h>
#include <stdio.h>

#include <Python.h>

typedef int(*CALLBACK)(const char*, va_list);

CALLBACK global_cb = vprintf;
static PyObject* global_pycb = NULL;

void register_function(CALLBACK cb) {
    global_cb = cb;
}

int fun(const char* fmt, va_list ap) {
    char tmp[255];
    vsnprintf(tmp, 255, fmt, ap);
    PyObject* tmp_string = Py_BuildValue("(s)", tmp);
    if(global_pycb && tmp_string){
        printf("Calling pycb\n");
        PyGILState_STATE state = PyGILState_Ensure();
        PyObject_CallObject(global_pycb, tmp_string);
        Py_DECREF(tmp_string);
        PyGILState_Release(state);
    } else {
        printf("No pycb registered..\n");
    }
    return 0;
}

static PyObject* register_function_2(PyObject* self, PyObject* args) {
    global_cb = fun;
    PyObject* callback;
    PyArg_ParseTuple(args, "O", &callback);
    if(PyCallable_Check(callback)) {
        printf("Arg was callable\n");
        Py_XINCREF(callback);
        global_pycb = callback;
    } else {
        printf("Arg was not callable\n");
    }
    Py_INCREF(Py_None);
    return Py_None;
}

void run_function(const char *arg1, ...) {
    va_list ap;
    va_start(ap, arg1);
    global_cb(arg1, ap);
    va_end(ap);
}

static PyMethodDef SpamMethods[] = {
    {"register_function_2",  register_function_2, METH_VARARGS,
     "Register function."},
    {NULL, NULL, 0, NULL}        /* Sentinel */
};

static struct PyModuleDef spammodule = {
    PyModuleDef_HEAD_INIT,
    "my_lib",   /* name of module */
    NULL, /* module documentation, may be NULL */
    -1,       /* size of per-interpreter state of the module,
                 or -1 if the module keeps state in global variables. */
    SpamMethods
};

PyMODINIT_FUNC
PyInit_libmy_lib(void)
{
    return PyModule_Create(&spammodule);
}

test_ctypes.py

from ctypes import *

libc = CDLL('libc.so.6')
my_lib = CDLL('build/libmy_lib.so')

from build.libmy_lib import register_function_2

reg = register_function_2
run = my_lib.run_function
run.argtypes = (c_char_p,)

# Call once with vprintf
run(b"Test %s\n", b"Test2")

def callback(*args):
    print(args)

reg(callback)

print("Before call")

run(b"Test %s\n", b"Test2")

NickeZ avatar Sep 26 '18 21:09 NickeZ

@NickeZ Before we go too crazy here, can we take a step back? Can you give actual code you are trying to use? That's not to say there might not be a problem, but let's try to define what it is that is trying to be solved. I doubt very much that having to write a C function to handle CAs printf() will be used very often. Is that what you want to do?

I think I have never found myself needing to overwrite the handler of printf(). Is it insufficient to use sys.stdout.write? Is it also not sufficient to overwrite sys.stdout.write (a sort of common python approach)? To be clear, I can believe so, but I can also believe that it is an unlikely use case.

I think the idea for replace_printf_handler() is that you should be able to supply any method like sys.stdout.write. Does sys.stdout.write work OK but not other functions?

Again, that's all to say that I think we should document the actual problem well before trying to solve it. For sure, adding C code that needs to be compiled would be a big change.

newville avatar Sep 27 '18 00:09 newville

Right, what I want to do is process the exception and not write it to stdout/err. Initially I was going to use ca_add_exception_event but pyepics doesn't implement it according to the docs.

Actually now that I read the pyepics documentation a little bit more careful it says that it is supposed to process the exceptions and turn them into python exceptions. This is not what happens to me. I think I get it printed out with the default handler.

import epics
import time
def onChange(value=None, **kw):
    print(value)
epics.PV('cagwtest_steady:0-waveform',auto_monitor=True, callback=onChange)
while True:
    time.sleep(0.1)
[  239.   240.   241. ...,  2284.  2285.  2286.]
CA.Client.Exception...............................................
    Warning: "Virtual circuit disconnect"
    Context: "pc11508.psi.ch:5064"
    Source File: ../cac.cpp line 1223
    Current Time: Thu Sep 27 2018 08:56:41.898663054
..................................................................

NickeZ avatar Sep 27 '18 07:09 NickeZ

Does sys.stdout.write work OK but not other functions?

No, if you do epics.ca.replace_printf_handler(None) or epics.ca.replace_printf_handler(sys.stdout.write) you get:

[  163.   164.   165. ...,  2208.  2209.  2210.]
Traceback (most recent call last):
  File "_ctypes/callbacks.c", line 234, in 'calling callback function'
TypeError: write() argument must be str, not bytes

And if you replace it with a callback that simply prints its arguments you get:

  189.   190.   191. ...,  2234.  2235.  2236.]
(b'CA.Client.Exception...............................................\n',)
(b'    %s: "%s"\n',)
(b'    Context: "',)
(b'localhost:5064',)
(b'"\n',)
(b'    Source File: %s line %d\n',)
(b'    Current Time: %s\n',)
(b'..................................................................\n',)

As you see the callback only receives the formatting string.

NickeZ avatar Sep 27 '18 07:09 NickeZ

@NickeZ It then sounds like there are sort of two problems.

Wrapping ca_add_exception_event sounds like what you really want to be able to do. Adding that seems reasonable to me. And not having such a wrapping is why the Virtual Circuit disconnects end up getting written out.

For using epics.ca.replace_printf_handler(my_function) what is the function you are replacing it with?

I think a common approach in Python would be to overwrite sys.stderr as with

logf  = open('/tmp/stderr_messages', 'w')
sys.stderr = logf

which will then send all stderr messages to that file. If you want them in a buffer you'd use io.BytesIO (or StringIO in Py2) instead of open. I have not tested that yet with CA exceptions. And admittedly, that approach would send all stderr messages (all uncaught exception messages) to that file.

Anyway, that approach is definitely a workaround to the actual problem. One thing I might (but again, have not yet) tried is

logf = open('/tmp/ca_exceptions.log', 'w')
epics.ca.replace_printf_handler(logf.write)

But again, maybe wrapping ca_add_exception_event is the better way to go?

Sorry that I don't have time in the next few weeks to jump into this.

newville avatar Sep 27 '18 11:09 newville

Yes I agree, what I actually would like is ca_add_exception_event. (The name seems to be misleading, according to the docs it works like something that should have been called ca_replace_exception_handler. But naming is one of the hardest thing in computer science...)

However, epics.ca.replace_printf_handler is currently broken and not useful. Additionally, I don't see a meaningful usecase for replacing a "printf handler" with a python function. Python doesn't have a concept of printf functions. You could technically give ca_replace_printf_handler a pointer to libc.vprintf. But you cannot use epics.ca.replace_printf_handler because it takes a python function.

logf = open('/tmp/ca_exceptions.log', 'w')
epics.ca.replace_printf_handler(logf.write)

This code won't do anything useful. First of all this code would throw exceptions on every call because you are trying to write byte sequences instead of strings. Secondly, the only argument write() recieves is the format string becuase the callback type is defined as (const char*) -> void instead of (const char*, va_list) -> int.

It seems to be technically impossible to define a callback type that takes va_list, which is why I suggested a wrapper implemented in C. The wrapper could be more elaborate and actually give you something like (str fmt, *args) which you then could try to format in python with fmt % args. I'm not sure that actually works out in practice though..

My point is, I don't think it is useful to replace a printf style function with a python function.

Let's keep this issue open as a tracking issue for implementing ca_add_exception_event.

NickeZ avatar Sep 27 '18 12:09 NickeZ

It works as Matt suggests if you do

logf = open('/tmp/ca_exceptions.log', 'wb')

which opens the file in binary mode so it expects bytes not str. However, the output looks like

CA.Client.Exception...............................................
    %s: "%s"
    Context: "sharon:49295"
    Source File: %s line %d
    Current Time: %s
..................................................................
CA client library is unable to contact CA repeater after %u tries.
Silence this message by starting a CA repeater daemon
or by calling ca_pend_event() and or ca_poll() more often.
CA.Client.Exception...............................................
    %s: "%s"
    Context: "sharon:64617"
    Source File: %s line %d
    Current Time: %s
..................................................................

tacaswell avatar Sep 27 '18 13:09 tacaswell

@NickeZ I think it's fine to expect that both ca_add_exception_event() should be supported and replace_printf_handler should work ("better" or perhaps even "at all"). It looks like these are each non-trivial problems, and might need some real work.

In particular, it seems strange to me that using sys.stderr.write appears to be ok but that a different file handle is not. I can believe it may be impossible to wrap a python function to appear to use C's va_list, but I don't know that for sure. And it seems really odd that using sys.stderr.write would be different from the write method on any other file handle. But I haven't looked into any of these issues in any detail....

newville avatar Sep 27 '18 14:09 newville

it seems strange to me that using sys.stderr.write appears to be ok

Are you aware that it would print strings like %s: "%s" to stderr? Is that "ok" to you?

NickeZ avatar Sep 28 '18 08:09 NickeZ

@NickeZ Oh, so it's not working with sys.stderr.write either? That actually makes more sense to me. I have not actually tried that recently.

For future reference, this is one of the reasons why it is always better to give a reproducible example, including complete error messages. We're a couple days into this discussion and I'm still trying to figure out the nature of the problem.

newville avatar Sep 28 '18 11:09 newville

Yeah, I'm sorry I didn't provide a reproducible example. Maybe an issue template could serve as a gentle reminder to future bug reporters.

A reduced test case to reproduce my initial problem is:

  1. Run the following script
epics.ca.replace_printf_handler()
epics.PV('a_pv', auto_monitor=True)
  1. Start and stop the IOC containing a_pv to force CA.Client.Exception.

error message is:

Traceback (most recent call last):
  File "_ctypes/callbacks.c", line 234, in 'calling callback function'
TypeError: write() argument must be str, not bytes

NickeZ avatar Sep 28 '18 12:09 NickeZ