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.

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

Mitigate race conditions in Adapters #141

Closed
dbemiller opened this issue Oct 3, 2017 · 3 comments
Closed

Mitigate race conditions in Adapters #141

dbemiller opened this issue Oct 3, 2017 · 3 comments
Assignees

Comments

@dbemiller
Copy link
Contributor

In this project's short lifetime, we've already started to see how easy it is to introduce race conditions (#122, #140).

This is a problem, because:

  1. The project has no automated way to catch them. Only code review & comments.
  2. Adapters are run in their own goroutine, so segfaults crash the entire process.

Programmers make mistakes, and anything which isn't caught automatically will happen in the future. As this project becomes more active, this will become a bigger issue.

There are a few options for how we can mitigate these issues.

  1. Copy every object we send into the Adapters' goroutines, so that data races aren't possible.
  2. Add "good adapter citizen" tests which run on all adapters, and make sure they don't mutate shared state.
  3. Change the Types we send into Adapter.Call() so that adapters can't mutate things they're not allowed to. For example, make the PBSRequest state private and expose methods which only return immutable data.

Anyone have preferences, or other ideas?

@mkendall07
Copy link
Member

I like option 3. Although 2 is probably also a good idea. 1 seems a bit heavy handed. That's all I got. no new ideas.

@dbemiller
Copy link
Contributor Author

There's also some overlap here with #150.

If the PBS external API changes to accept openRTB requests, then the pbsrequest type will go away, and the adapter API will have to change.

@dbemiller
Copy link
Contributor Author

#356 should (I hope) make Travis catch many of these. If we still find that we're running into problems, we can look into ways to improve the design farther.

StarWindMoonCloud pushed a commit to ParticleMedia/prebid-server that referenced this issue Jun 5, 2024
Adding new crid to be blocked this week for prebid
scr-oath added a commit to scr-oath/prebid-server that referenced this issue Nov 15, 2024
* New Adapter: MeloZen (prebid#3784)

* taboola-support-app-in-prebid-server (prebid#3795)

* OwnAdx: Bidder param and URL updates (prebid#3813)

Co-authored-by: Hina Yadav <hina.yadav@vertoz.com>

* Use format=prebid on adserver requests. (prebid#3846)

* New Module: 51Degrees (prebid#3650)

Co-authored-by: James Rosewell <james@51degrees.com>
Co-authored-by: Marin Miletic <mrnmiletic@gmail.com>
Co-authored-by: Sarana-Anna <anna.sarana@gmail.com>
Co-authored-by: Eugene Dorfman <eugene.dorfman@gmail.com>
Co-authored-by: Krasilchuk Yaroslav <legend.ko@hotmail.com>

* ADTS-455 remove video validations (prebid#3842)

authored by @gg-natalia

* Update Vidazoo bidder info for GPP support (prebid#3869)

* declare support for ORTB 2.6 (prebid#3872)

authored by @bretg

* new adapter (prebid#3833)

authored by @Pubrise

* Fix currency conversion bug. (prebid#3867)

Co-authored-by: ddubyk <ddubyk@magnite.com>

* freewheel-adapter: support 2.6 (prebid#3873)

* Update mobilefuse.yaml to indicate support for OpenRTB 2.6 and GPP (prebid#3871)

* specifies ortb 2.6 support (prebid#3) (prebid#3876)

* PulsePoint: ortb 2.6 version and gpp support (prebid#3874)

authored by @anand-venkatraman

* Revert "New Module: 51Degrees (prebid#3650)" (prebid#3888)

This reverts commit 2606e75.

---------

Co-authored-by: benben2001 <145416009+benben2001@users.noreply.github.com>
Co-authored-by: ahmadlob <109217988+ahmadlob@users.noreply.github.com>
Co-authored-by: ownAdx <135326256+ownAdx-prebid@users.noreply.github.com>
Co-authored-by: Hina Yadav <hina.yadav@vertoz.com>
Co-authored-by: Antonios Sarhanis <tsarhanis@gmail.com>
Co-authored-by: James Rosewell <james@51degrees.com>
Co-authored-by: Marin Miletic <mrnmiletic@gmail.com>
Co-authored-by: Sarana-Anna <anna.sarana@gmail.com>
Co-authored-by: Eugene Dorfman <eugene.dorfman@gmail.com>
Co-authored-by: Krasilchuk Yaroslav <legend.ko@hotmail.com>
Co-authored-by: gg-natalia <148577437+gg-natalia@users.noreply.github.com>
Co-authored-by: Saar Amrani <saar120@gmail.com>
Co-authored-by: bretg <bgorsline@gmail.com>
Co-authored-by: Pubrise <prebid@pubrise.ai>
Co-authored-by: Dubyk Danylo <45672370+CTMBNara@users.noreply.github.com>
Co-authored-by: ddubyk <ddubyk@magnite.com>
Co-authored-by: mwang-sticky <mwang@freewheel.tv>
Co-authored-by: dtbarne <7635750+dtbarne@users.noreply.github.com>
Co-authored-by: Nick Llerandi <nick.llerandi@kargo.com>
Co-authored-by: Anand Venkatraman <avenkatraman@pulsepoint.com>
Co-authored-by: Brian Sardo <1168933+bsardo@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants