On Thu, Jul 11, 2019 at 9:27 AM Nitish Saboo <nitish.sabo...@gmail.com> wrote:
>
> Following is my C Code:
>
> PatternDB *patterndb[WORKERIDS];
>
> int load_pattern_db(const gchar* filename, key_value_cb cb, int workerId)
> {
> if(patterndb[workerId] != NULL){
> pattern_db_free(patterndb[workerId]);
> }
> patterndb[workerId] = pattern_db_new();
> pattern_db_reload_ruleset(patterndb[workerId], configuration, filename);
> pattern_db_set_emit_func(patterndb[workerId], pdbtool_pdb_emit_accumulate,
> cb);
> return 0;
> }
>
> This is just the C wrapper that internally call syslog-ng methods.
>
> Since I am using an array of patterndbs, I am calling the C code with
> multiple Go routines.
>
> Single iteration of the code is working fine.
>
> The issue is seen only with multiple go routines. Can we store the address of
> the C struct (when memory is allocated using calloc) on Go side in an array
> so that I can deallocate the memory using 'defer
> C.free(unsafe.Pointer(InitStruct))' when the callback is done?This should
> make multiple go routines to work...Am I right ?
I don't know what this code does, but the line
pattern_db_set_emit_func(patterndb[workerId], pdbtool_pdb_emit_accumulate, cb);
suggests that cb is being saved away for future use. If that is
indeed the case then freeing the memory in Go will cause a dangling
pointer, leading to crashes as you see.
You should also double-check that each separate goroutine is using a
different "workerId", as duplication of workerId values could
certainly cause trouble when running concurrently.
Yes, it's fine to store the address of the C struct on the Go side, if
that helps.
Ian
> On Thu, Jul 11, 2019 at 9:39 PM Ian Lance Taylor <i...@golang.org> wrote:
>>
>> On Thu, Jul 11, 2019 at 7:16 AM Nitish Saboo <nitish.sabo...@gmail.com>
>> wrote:
>> >
>> > I have the following function 'LoadPatternDB' where I am allocating memory
>> > for a C struct.My C struct contains the callback function.
>> >
>> > 1)I cannot deference the struct in the same func 'LoadPatternDB' because I
>> > get the following error when my callback function is called from C code:
>> >
>> > signal SIGSEGV: segmentation violation code=0x1 addr=0x7f1e00415341
>> > pc=0x7f1e00415341]
>> >
>> > 2)The same code is running in multiple go routines.And because of that if
>> > I don't deallocate the memory I get the following error:
>> >
>> > *** Error in `fatal error: bin/main': double free or corruption
>> > (fasttop)unexpected signal during runtime execution: 0x00007fd2b8588060 ***
>> >
>> > My question is:
>> >
>> > How can I store the address of the C struct (when memory is allocated
>> > using calloc) on Go side in an array so that I can deallocate the memory
>> > using 'defer C.free(unsafe.Pointer(InitStruct))' when the callback is done.
>> >
>> >
>> > 'InitStruct := (*C.Accumulatedparams)(C.calloc(1,
>> > C.sizeof_struct_Accumulatedparams))' >>>> I want to store this address on
>> > Go side.
>> >
>> >
>> > Can some please guide me here ?
>> >
>> >
>> > syslogparser.go
>> > =============
>> >
>> > func (obj SyslogParser) LoadPatternDB(opts Syslog, workerId int) {
>> > patterndbpath := C.CString(opts.Patterndb)
>> > defer C.free(unsafe.Pointer(patterndbpath))
>> > InitStruct := (*C.Accumulatedparams)(C.calloc(1,
>> > C.sizeof_struct_Accumulatedparams))
>> > //defer C.free(unsafe.Pointer(InitStruct)) . <<<<<<<<<<<<<<<<<<<<<<I
>> > cannot do this here
>> > InitStruct.callback = (C.key_value_cb)(C.callback)
>> > InitStruct.data = C.int(workerId)
>> > C.load_pattern_db(patterndbpath,
>> > (*C.Accumulatedparams)(unsafe.Pointer(InitStruct)), C.int(workerId))
>> > }
>> >
>> >
>> > //export Result
>> > func Result(key *C.char, value *C.char, value_len C.size_t, mapid C.int) {
>> > fmt.Println("Map Id: " + strconv.Itoa(int(mapid)))
>> > value_field := C.GoStringN(value, C.int(value_len))
>> > key_field := C.GoString(key)
>> > remap, ok := constants.FIELD_MAPPINGS[key_field]
>> > if ok {
>> > Check.result[int(mapid)][remap] = value_field
>> > //check.result[remap] = value_field
>> > } else {
>> > //check.result[key_field] = value_field
>> > Check.result[int(mapid)][key_field] = value_field
>> > }
>> > }
>> >
>> >
>> > cfuncs.go
>> > -========
>> > package lib
>> >
>> > /*
>> >
>> > #include <stdio.h>
>> >
>> > // The gateway function
>> > void callback(char *key, char *value, size_t value_len, int mapid)
>> > {
>> > //printf("C.callOnMeGo_cgo(): called");
>> > void Result(const char *key, const char *value, size_t value_len, int
>> > mapid);
>> > Result(key, value, value_len, mapid);
>> > }
>> > */
>> > import "C"
>> >
>> > syslog-node.h
>> > ============
>> >
>> > #ifndef TEST_H_INCLUDED
>> > #define TEST_H_INCLUDED
>> >
>> > #include <stdlib.h>
>> >
>> > typedef void (*key_value_cb)(const char* key, const char* value, size_t
>> > value_len, int data);
>> > typedef struct Accumulatedparams{
>> > key_value_cb callback;
>> > int data;
>> > }Accumulatedparams;
>> > int initialize_engine(const char* filename, const char* module_path);
>> > int reload_pattern_db(const char* filename, Accumulatedparams *cb, int
>> > workerId);
>> > int load_pattern_db(const char* filename, Accumulatedparams *cb, int
>> > workerId);
>> >
>> > #endif
>>
>>
>> I don't see anything obviously wrong in your Go code, but it's
>> impossible to answer your question without knowing what the C code
>> does. What you describe is consistent with C code that expects to
>> retain the memory passed to load_pattern_db, or alternatively is also
>> consistent with C code that does not expect to be called concurrently
>> from multiple threads of execution.
>>
>> Ian
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/golang-nuts/CAOyqgcU4jBvOMHqTZ%3DhyePT23Tb3DEhHhaydu6%2Bn51GW2J3Bbw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
