Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Gateway may panic when loading duplicate APIs on stable #938

Closed
mvdan opened this issue Jul 24, 2017 · 2 comments
Closed

Gateway may panic when loading duplicate APIs on stable #938

mvdan opened this issue Jul 24, 2017 · 2 comments
Assignees
@mvdan
Milestone
Release 2.3.8

Comments

@mvdan
Copy link
Contributor

mvdan commented Jul 24, 2017 

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x42cfbb]
goroutine 405 [running]:
panic(0xccaae0, 0xc420010080)
/usr/local/go/src/runtime/panic.go:500 +0x1a1
main.processSpec(0xc42266c280, 0xc420411b30, 0x11, 0xc4252ae140, 0xc4252ae160, 0xc4252ae180, 0xc4253d0480, 0xc4253d04e0, 0xc420411b30, 0xc42003eee8)
/src/github.com/TykTechnologies/tyk/api_loader.go:149 +0x5bb
main.loadApps.func1(0xc4252a9a30, 0xc4200218d8, 0xc4252ae140, 0xc4252ae160, 0xc4252ae180, 0xc4253d0480, 0xc4253d04e0, 0xc4253d0540, 0xc42266c280, 0x11, ...)
/src/github.com/TykTechnologies/tyk/api_loader.go:646 +0x10a
created by main.loadApps
/src/github.com/TykTechnologies/tyk/api_loader.go:649 +0x4eb

That line contains:

prev := (*ApiSpecRegister)[referenceSpec.APIID]

Which was added in 2.3.6 as part of deduplicating APIs when loading them.

I'm fairly sure the panic refers to ApiSpecRegister being nil, and not referenceSpec, as the latter is used a few lines further up in the codebase. And ApiSpecRegister can be nil sometimes, as a pointer to a map.

The good news is that this is already fixed in master, as we got rid of the double pointer. We should do something similar in stable.
@mvdan mvdan self-assigned this Jul 24, 2017
@mvdan mvdan added this to the Release 2.3.8 milestone Jul 24, 2017
@mvdan
Copy link
Contributor Author

mvdan commented Jul 24, 2017

To clarify - the bug was introduced when backporting this feature to stable. We overlooked the situation with the pointer to the map.

@mvdan mvdan mentioned this issue Jul 24, 2017
Merged
buger pushed a commit that referenced this issue Jul 26, 2017
@mvdan @buger
all: don't make the global api spec map a pointer
dc28342 
It's unnecessary, as maps are already pointers. This was done in master,
but not yet in stable.

The tricky part is that the double-pointer means that reads can't be
safely done without checking for nil first, which can be done if the
double pointer is removed.

This was missed in the backport of the API deduplication feature, which
led to a nil pointer dereference crash.

Fixes #938.
@mvdan
Copy link
Contributor Author

mvdan commented Jul 26, 2017

This bug only affects stable, and it was merged. GitHub won't close unless there's a merge in master, but there won't be one. Closing manually.

@mvdan mvdan closed this as completed Jul 26, 2017
@mvdan mvdan mentioned this issue Aug 29, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mvdan mvdan

Labels
None yet
Projects
None yet
Milestone
Release 2.3.8
Development

No branches or pull requests
1 participant
@mvdan