Re: String size limit for Eiger FileWriter when compiling with g++ > 5.2

["Wlodek, Jakub via Tech-talk" <tech-talk at aps.anl.gov>]

Hi Andrew,

Thanks for the response, I made a pull request with the fix here:

https://github.com/areaDetector/ADEiger/pull/35

When I discovered this issue, my thought was that it was due to an incompatibility with the C++ string type and the

C type data structures powering libcom, as you said.

Best,

Jakub

---

From: Johnson, Andrew N. <anj at anl.gov>
Sent: Wednesday, March 4, 2020 1:37 PM
To: Wlodek, Jakub <jwlodek at bnl.gov>
Cc: Bruno Martins <brunoseivam at gmail.com>; EPICS tech-talk <tech-talk at aps.anl.gov>
Subject: Re: String size limit for Eiger FileWriter when compiling with g++ > 5.2

 

Hi Jakub,

Never put a std::string (or any other C++ standard library template object) onto an epicsMessageQueue, as Bruno said. Your fix of replacing that with a fixed-size char array is probably the best solution for now, the libCom APIs are mostly designed for C data types and POD (Plain Old Data) structures. Wherever you found that code, please file a bug report against the module.

Thanks,

- Andrew

On Mar 4, 2020, at 12:23 PM, Wlodek, Jakub via Tech-talk <tech-talk at aps.anl.gov> wrote:

Hi Bruno,

Thanks for letting me know, that makes sense. I think that the current fix is probably OK for now, it only required changing about 10 lines, so it wasn't overly complex.

I wonder if there are any other instances of EPICS drivers that would suffer from a similar problem, with std::string memcpy calls to put them on a queue.

Best,

Jakub

---

From: Bruno Martins <brunoseivam at gmail.com>
Sent: Wednesday, March 4, 2020 1:19 PM
To: Wlodek, Jakub <jwlodek at bnl.gov>
Cc: tech-talk at aps.anl.gov <tech-talk at aps.anl.gov>
Subject: Re: String size limit for Eiger FileWriter when compiling with g++ > 5.2

 

I believe memcpy'ing a std::string might not be safe. My theory is that std::string internals changed: the underlying data is probably being automatically deallocated between sending a string on one end and getting it on the other end of a queue, since we're essentially copying bytes. It might not happen for smaller strings because their content is probably inlined in the std::string type itself.

Maybe that was always the behavior, but the inlining length threshold is the thing that changed.

The way I see it, you have to either keep the change you mentioned OR use a different queue that's aware of the data type it is transporting, taking ownership of the queue items (which, AFAIK, EPICS doesn't provide).

Bruno

On Wed, Mar 4, 2020 at 12:12 PM Wlodek, Jakub via Tech-talk <tech-talk at aps.anl.gov> wrote:

Hello all,

I have an interesting issue that I wanted to get some feedback on. One of our beamlines is running several Eiger detectors that were recently moved to a Debian 9 system,

and as soon as they were moved, we recieved feedback that the FileWriter was not saving the h5 files to the local storage.

After some testing, this was confirmed, however, only when the FilePattern PV was longer than 15 characters. The h5 file would be saved to the Eiger server and was visible

through the web interface, but was not copied to the local storage. Seeing as 15 characters was the limit, and the call sizeof(std::string) would return 15 with g++ > 5.2, we figured 

out that somewhere there was an issue stemming from this, especially since on Debian 7 (g++ version 4.7.2) we did not see the problem.

I found that the following structure is sent on a message queue: epicsMessageQueue mPollQueue:

typedef struct

{

  string pattern;

    int sequenceId;

    size_t nDataFiles;

     bool saveFiles, parseFiles, removeFiles;

     mode_t filePerms;

}acquisition_t;

And calling pattern.c_str() before it was sent on the queue would print the full value, but on the recieve pattern.c_str() would be empty whenever the string

was longer than 15 characters. The queue was initialized with:

mPollQueue(1, sizeof(acquisition_t)),

My first idea was to simply increase the size of this second parameter for the epicsMessageQueue, but that did not solve the problem.

What did was changing the string pattern to char pattern[MAX_BUF_SIZE], and ensure that any time this pattern variable was called a conversion from string

to char[] was done.

My question is, is there a better way to fix this where the driver can continue to use std::string? And why did increasing the max transport size for the queue not

also fix the issue?

Regards,

Jakub

-- 

Complexity comes for free, Simplicity you have to work for.