Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

bug in ReadTopologyInstanceBufferable #997

Closed
yangeagle opened this issue Oct 19, 2019 · 2 comments · Fixed by #1034
Closed

bug in ReadTopologyInstanceBufferable #997

yangeagle opened this issue Oct 19, 2019 · 2 comments · Fixed by #1034

Comments

@yangeagle
Copy link
Contributor

When I start GracefulMasterTakeover, a panic occured:

2019-10-17 14:08:23 ERROR ReadTopologyInstance(abc-mysql:3306) Unexpected, aborting: runtime error: invalid memory address or nil pointer dereference
[martini] PANIC: runtime error: invalid memory address or nil pointer dereference
/usr/lib/golang/src/runtime/panic.go:513 (0x432908)
        gopanic: reflectcall(nil, unsafe.Pointer(d.fn), deferArgs(d), uint32(d.siz), uint32(d.siz))
/usr/lib/golang/src/runtime/panic.go:82 (0x431a5d)
        panicmem: panic(memoryError)
/usr/lib/golang/src/runtime/signal_unix.go:390 (0x448831)
        sigpanic: panicmem()
/tmp/build/src/orchestrator/.gopath/src/github.com/github/orchestrator/go/logic/topology_recovery.go:1843 (0x909b45)
/tmp/build/src/orchestrator/.gopath/src/github.com/github/orchestrator/go/http/api.go:3035 (0x963f58)
/tmp/build/src/orchestrator/.gopath/src/github.com/github/orchestrator/go/http/api.go:3691 (0x97f8f5)

I found the crash line is:
https://github.com/github/orchestrator/blob/5c80b6aaba9ba83fa8c1fe8bf8fe6df99e7c4d24/go/logic/topology_recovery.go#L1843

The reason is: masterOfDesignatedInstance is nil.

I look into the definition of masterOfDesignatedInstance.

And the line below calls
ReadTopologyInstanceBufferable at last:
https://github.com/github/orchestrator/blob/5c80b6aaba9ba83fa8c1fe8bf8fe6df99e7c4d24/go/logic/topology_recovery.go#L1839

A panic below occured in ReadTopologyInstanceBufferable, but not return to the caller:

Unexpected, aborting: runtime error: invalid memory address or nil pointer dereference

The code of ReadTopologyInstanceBufferable:

func ReadTopologyInstanceBufferable(instanceKey *InstanceKey, bufferWrites bool, latency *stopwatch.NamedStopwatch) (*Instance, error) {
	defer func() {
		if err := recover(); err != nil {
			logReadTopologyInstanceError(instanceKey, "Unexpected, aborting", fmt.Errorf("%+v", err))
		}
	}()

... ...

The err modified in defer will not return to the caller.

The same bug as #943.

Please check it.

cc @MOON-CLJ

@yangeagle
Copy link
Contributor Author

@shlomi-noach I have a PR to fix this problem.

@shlomi-noach
Copy link
Collaborator

@yangeagle sorry for the late response. I'm happy for a PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants