pattern_db_set_emit_func(patterndb[workerId], pdbtool_pdb_emit_accumulate,
cb);
suggests that cb is being saved away for future use.

>>>>yes cb is being saved away and is being used for future use for
callbacks to Go code.

Yes, it's fine to store the address of the C struct on the Go side, if
that helps.

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);

>>>>>If I declare the array in the following manner:

result := make([]*C.struct_Accumulatedparams)
store[0] = (*C.Accumulatedparams)(C.calloc(1,
C.sizeof_struct_Accumulatedparams))

*Here also the code gets compiled successfully, just that the GoLand IDE
shows **store[0].callback and **store[0].data in red color saying **Unresolved
reference 'callback' and Unresolved reference 'data' respectively.Not sure
how the code compiles if the IDE shows it in red color. Binary(./main)
works fine*

Though this works,

store[0] = (*C.Accumulatedparams)(C.calloc(1,
C.sizeof_struct_Accumulatedparams))
InitStruct := store[0]
InitStruct.callback = (C.key_value_cb)(C.callOnMeGo_cgo)
InitStruct.data = C.int(1)

*Here the code compiles successfully and there is no complain from GoLand
IDE as well.Binary (./main) too works fine*

So what i thought to do :

type Store struct{
result []*C.struct_Accumulatedparams
data []C.int
callback []C.key_value_cb
}

var store = Store{result: make([]*C.struct_Accumulatedparams, 2), data:
make([]C.int ,2), callback:make([]C.key_value_cb, 2)}

func (obj Syslogparser) LoadPatternDB(opts Syslog) {

patterndb_path := C.CString(opts.patterndb)
store.result[0] = (*C.Accumulatedparams)(C.calloc(1,
C.sizeof_struct_Accumulatedparams)) <<<<<<
store.callback[0] = (C.key_value_cb)(C.callOnMeGo_cgo) <<<<<
store.data[0] = C.int(1)
C.load_pattern_db(patterndb_path,
(*C.Accumulatedparams)(unsafe.Pointer(store.result[0])))
}

*Here the code gets compiled successfully. But I am getting the following
error when I run the binary './main'*

1In reload PatternDb:
PatternDb :/home/nsaboo/Documents/goworkspace/src/poc/reload.xml
ModulePath :usr/local/lib/syslog-ng
fatal error: unexpected signal during runtime execution
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x0]

runtime stack:
runtime.throw(0x4fa203, 0x2a)
/usr/local/go/src/runtime/panic.go:617 +0x72
runtime.sigpanic()
/usr/local/go/src/runtime/signal_unix.go:374 +0x4a9

goroutine 1 [syscall]:
runtime.cgocall(0x4b8fb0, 0xc000042cc8, 0x4d9280)
/usr/local/go/src/runtime/cgocall.go:128 +0x5b fp=0xc000042c98
sp=0xc000042c60 pc=0x404f6b
main._Cfunc_execute_percustomer_db(0x7fb0fb5a5fa0, 0x1d5, 0x7fb0fb8259b0,
0x8)
_cgo_gotypes.go:121 +0x45 fp=0xc000042cc8 sp=0xc000042c98 pc=0x4b5ca5
main.Syslogparser.ParseMessagePerCustomer(0xc0000a2000, 0x36, 0x4f6b0c,
0x18, 0x4f3fc4, 0x8, 0xc0000d0000, 0x1d5)
/home/nsaboo/Documents/goworkspace/src/poc/main.go:132 +0x109
fp=0xc000042d38 sp=0xc000042cc8 pc=0x4b6f09
main.perCustomerParsing(...)
/home/nsaboo/Documents/goworkspace/src/poc/main.go:249
main.main()
/home/nsaboo/Documents/goworkspace/src/poc/main.go:232 +0x6e1
fp=0xc000042f98 sp=0xc000042d38 pc=0x4b78e1
runtime.main()
/usr/local/go/src/runtime/proc.go:200 +0x20c fp=0xc000042fe0
sp=0xc000042f98 pc=0x42cd9c
runtime.goexit()
/usr/local/go/src/runtime/asm_amd64.s:1337 +0x1 fp=0xc000042fe8
sp=0xc000042fe0 pc=0x4548c1
}


*Where am i going wrong here ?*

Thanks,
Nitish

On Thu, 11 Jul 2019, 23:03 Ian Lance Taylor, <i...@golang.org> wrote:

> 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/CALjMrq5g1uNN-gFzL_GJ7sEqeYyGvwtbRAAJ-c3%3DfRSGfzaNAw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to